Вопрос по refactoring, java, if-statement – Как избежать многих условий

35

Я прочитал много тем о рефакторинге кода и избегании операторов if else. На самом деле, у меня есть класс, в котором я использую множество условий if-else.

Более подробно: я использую парсер pull, и в каждой строке моего мыльного ответа я буду проверять, есть ли интересующий меня тег, если нет, проверять другой тег и т. Д .:

<code> if(eventType == XmlPullParser.START_TAG) {
            soapResponse= xpp.getName().toString();

            if (soapResponse.equals("EditorialOffice")){  
                eventType = xpp.next();
                if (xpp.getText()!=null){
                editorialOffice += xpp.getText();
                }
            }   
            else if (soapResponse.equals("EditorialBoard")){  
                eventType = xpp.next();
                if (xpp.getText()!=null){
                editorialBoard += xpp.getText();
                }
            }
            else if (soapResponse.equals("AdvisoryBoard")){  
                eventType = xpp.next();
                if (xpp.getText()!=null){
                advisoryBoard += xpp.getText();
                }
            }   
        }
        eventType = xpp.next();
     }
</code>

Теперь я хотел бы использовать что-то другое вместо тех, если еще условия, но я не знаю что.

Можете ли вы дать мне пример или хорошую страницу учебника?

Спасибо.

StringBuilder sb = new StringBuilder(); sb.append(editorialBoard); sb.append(xpp.getText); editorialBoard=sb.toString(); KarlM
Примечание: вы должны использовать StringBuilder вместо + = для объединения ваших строк в целях повышения производительности. Sam Barnum
Спасибо Сэм Барнум, как использовать строителя строк? Я постараюсь проверить в Интернете, но пример будет отличным. Miloš
Проверь это:stackoverflow.com/questions/519422/…    надеюсь, что это полезно Android Stack
Вы можете поддерживать отображение строк в перечисления в других местах программы, извлекать перечисление, связанное с возвращенной строкой, из карты (по умолчаниюNO_MATCH в случае, если строка отсутствует на карте), и напишите оператор switch в перечислениях. Это может сделать этот код более понятным за счет дополнительного уровня косвенности. Вам придется судить, стоит ли это того. Chris Taylor

Ваш Ответ

8   ответов
1

Вы можете определить перечисление следующим образом:

public enum SoapResponseType {
    EditorialOffice(1, "description here") {
        public void handle(XmlPullParser xpp) {
            //do something you want here
            return null;
        }
    },
    EditorialBoard(2, "description here") {
        public void handle(XmlPullParser xpp) {
            //do something you want here
            return null;
        }
    },
    AdvisoryBoard(3, "description here") {
        public void handle(XmlPullParser xpp) {
            //do something you want here
            return null;
        }
    };

    public static SoapResponseType nameOf(String name) {
        for (SoapResponseType type : values()) {
            if (type.getName().equalsIgnoreCase(name)) {
                return type;
            }
        }
        return null;
    }

    public void handle(XmlPullParser xpp) {
        return null;
    }
}

Используйте перечисление выше, как это:

SoapResponseType type = SoapResponseType.nameOf("input string");
if (type != null) {
    type.handle(xpp);
}

Это чистый код, не правда ли!

8

В Java 7 вы можете переключаться на строки. Вы могли бы использовать это, если бы вы могли использовать это ;-)

+1 для & quot; вы могли бы использовать это, если бы вы могли использовать это. & Quot; Если бы у нас были яйца, мы могли бы иметь ветчину и яйца, если бы у нас была ветчина.
Если вы пытаетесь сделать код более чистым, переключатель не сделает его намного чище
25

В этом конкретном случае, поскольку код практически идентичен для всех трех случаев, кроме строки, к которой добавляется строка, у меня будет запись карты для каждой из строящихся строк:

Map<String,String> map = new HashMap<String,String>();
map.put("EditorialOffice","");
map.put("EditorialBoard","");
map.put("AdvisoryBoard","");
// could make constants for above Strings, or even an enum

а затем измените свой код на следующий

if(eventType == XmlPullParser.START_TAG) {
    soapResponse= xpp.getName().toString();
    String current = map.get(soapResponse);
    if (current != null && xpp.getText()!=null) {
        map.put( soapResponse, current += xpp.getText());
    }
    eventType = xpp.next();
}

Нет "если ... тогда ... еще". Даже не сложность нескольких классов для шаблонов стратегий и т. Д. Карты - ваш друг. Стратегия хороша в некоторых ситуациях, но эта достаточно проста, чтобы ее можно было решить.

4

Вы не упомянули, можете ли вы использовать Java 7. Вы можете использовать эту версию Java.Строки в операторах switch.

Помимо этого, инкапсуляция логики для каждого случая является хорошей идеей, например:

Map<String, Department> strategyMap = new HashMap<String, Department>();
strategyMap.put("EditorialOffice", new EditorialOfficeDepartment());
strategyMap.put("EditorialBoard", new EditorialBoardDepartment());
strategyMap.put("AdvisoryBoard", new AdvisoryBoardDepartment());

Затем вы можете просто выбрать правильную стратегию на карте и использовать ее:

String soapResponse = xpp.getName();
Department department = strategyMap.get(soapResponse);
department.addText(xpp.getText());

Department конечно в интерфейсе ...

После всех перерывов в работе я, наконец, отправил ответ, но в то же время @hwcverwe уже представил практически идентичный ответ (см. Выше).
5

Помимо комментариев zzzzzzz (и т. Д.) ... имейте в виду, что вы используете XmlPullParser, который заставляет вас писать некрасивый код, подобный тому, который у вас есть. Вы можете зарегистрировать некоторые обратные вызовы, которые разделят ваш код и сделают его «лучше», но, если возможно, просто используйте библиотеку SimpleXML или аналогичную.

Кроме того, вы можете реорганизовать свой код, чтобы сделать его более читабельным и менее подробным. Например, почему вы звонитеxpp.next() внутри каждого оператора if? Почему бы просто не позвонить на улицу хотя бы один раз:

if(eventType == XmlPullParser.START_TAG) {
    soapResponse= xpp.getName().toString();
    if (soapResponse.equals("EditorialOffice") && xpp.getText()!=null){  
        editorialOffice += xpp.getText();
    }   
    else if (soapResponse.equals("EditorialBoard") && xpp.getText()!=null){  
        editorialBoard += xpp.getText();
    }
    else if (soapResponse.equals("AdvisoryBoard") && xpp.getText()!=null){  
        advisoryBoard += xpp.getText();
    }   
}
eventType = xpp.next();
Спасибо @Cristian, На самом деле, я получаю xml-ответ от сервера, я не знаю, смогу ли я использовать что-то еще, что так просто, как XML-парсер. Я вызываю xpp.next () для каждого оператора, в то время как, когда искомый начальный тег найден, на следующей строке я найду нужную мне переменную и поместу ее в локальную переменную. Miloš
ТакжеXmlPullParser.getName() возвращаетсяStringтак что звонить не нужно.toString() в теме.
Документация довольно полная. Как только вы все настроите, это будет так же просто, как делать такие вещи, как:SomeObject oneObject = Xml.parse(SomeObject.class, xmlPayload);
@ Ана, просто взгляни на библиотеку SimpleXML; это сделает вашу жизнь легче и счастливее.
@ Кристиан, еще раз спасибо, у тебя есть пример, как это использовать, пожалуйста? Miloš
4

огромный вопрос, который этот, и нет никакого реального ответа. (и я не использую мыло очень часто)

Вот только некоторые идеи, основанные на вашем коде:

сначала вы можете группировать дубликаты кода

if (soapResponse.equals("EditorialOffice")
||soapResponse.equals("EditorialBoard")
||soapResponse.equals("AdvisoryBoard")){ 

Еще одна хорошая вещь, которую вы можете сделать, это поиграть с переключателями:

switch(soapResponse){
case "EditorialOffice":
case "EditorialBoard":
case "AdvisoryBoard":
eventType = xpp.next();
                if (xpp.getText()!=null){
                advisoryBoard += xpp.getText();
                }
break;

Также вы должны рассмотреть возможность разбить тест на маленькие функции:

public bool interestingTag(string s){
return (soapResponse.equals("EditorialOffice")
    ||soapResponse.equals("EditorialBoard")
    ||soapResponse.equals("AdvisoryBoard"));
}

    public processData(xpp){
    eventType = xpp.next();
                    if (xpp.getText()!=null){
                    editorialBoard += xpp.getText();
                    }
    ....}

Так что вы можете просто обработать все ваши ответы в цикле while, и вы будете очень длинны, если все еще станет функцией 5 ~ 10 строк

Но, как я уже сказал, есть много хороших способов сделать то же самое

3

Вы можете создать интерфейс ResponseHandler с тремя реализациями, по одной для каждой ветви вашей конструкции if / else.

Затем создайте карту, отображающую различные soapResponses на обработчик, или список со всем обработчиком, если он может обработать этот soapResponse.

Вы также должны быть в состоянии переместить часть стандартного кода в общую, возможно, абстрактную реализацию класса обработчика ответа.

Как часто бывает много вариантов этого. Используя дублирование кода, на самом деле нужна только одна реализация:

class ResponseHandler{
    String stringToBuild = "" // or what ever you need
    private final String matchString

    ResponseHandler(String aMatchString){
        matchString = aMatchString
    }
    void handle(XppsType xpp){
        if (xpp.getName().toString().equals(matchString){
            eventType = xpp.next();
            if (xpp.getText()!=null){
                 editorialOffice += xpp.getText();
            }
        }
    }
}

Ваш код становится

List<ResponseHandler> handlers = Arrays.asList(
    new ResponseHandler("EditorialOffice"),
    new ResponseHandler("EditorialBoard"),
    new ResponseHandler("AdvisoryBoard"));
if(eventType == XmlPullParser.START_TAG) {
    for(ResponseHandler h : handlers)
        h.handle(xpp);
}
Спасибо @Jean Sxhauder, не могли бы вы дать мне пример, как это использовать? Miloš
добавлен пример реализации
43

Попробуйте взглянуть на шаблон стратегии.

Make an interface class for handling the responses (IMyResponse) Use this IMyResponse to create AdvisoryBoardResponse, EditorialBoardResponse classes Create an dictionary with the soapresponse value as key and your strategy as value Then you can use the methods of the IMyResponse class by getting it from the dictionary

Маленький пример:

// Interface
public interface IResponseHandler {
   public void handleResponse(XmlPullParser xxp);

}

// Concrete class for EditorialOffice response
private class EditorialOfficeHandler implements IResponseHandler {
   public void handleResponse(XmlPullParser xxp) {
       // Do something to handle Editorial Office response
   }
}

// Concrete class for EditorialBoard response
private class EditorialBoardHandler implements IResponseHandler {
   public void handleResponse(XmlPullParser xxp) {
       // Do something to handle Editorial Board response
   }
}

На месте нужно создать обработчики:

Map<String, IResponseHandler> strategyHandlers = new HashMap<String,IResponseHandler>();
strategyHandlers.put("EditorialOffice", new EditorialOfficeHandler());
strategyHandlers.put("EditorialBoard", new EditorialBoardHandler());

Где вы получили ответ:

IResponseHandler responseHandler = strategyHandlers.get(soapResponse);
responseHandler.handleResponse(xxp);
+1: я сделал очень много, как вы предлагаете, для программы, которая обрабатывает приложение XML.
Спасибо @hwcverwe, мне кажется, это отличная идея. Не могли бы вы дать мне более точный пример, как это сделать? Большое спасибо. Miloš
Возможно, это подходит для более сложных многозадачных решений. Но разве это не слишком тяжело для этого конкретного случая? Черт, я бы даже предпочел "если бы, тогда". еще, если & quot; над этим решением, в данном конкретном случае (хотя существуют и другие более простые решения).
Я не утверждаю, что Стратегия слишком сложна. Кажется, в этом случае желаемый результат должен был добавить к текущей строке, выбранной с помощью селектора. Бывают моменты, когда стратегия является очевидным выбором, т. Е. Когда действие изменяется в зависимости от селектора. Если ситуация автора с 15 условиями if-else схожа, я не хотел бы реализовывать 15 классов для идентичных действий, и все, что отличалось, было целью, над которой работали. Если существует вероятность того, что действие будет отличаться в зависимости от выбора, тогда Стратегия поможет приспособиться к ожидаемым изменениям, и это станет отличным выбором.
@KevinWelker это немного сложнее, но если вы думаете о ремонтопригодности, это будет намного проще в будущем. очень легко расширить стратегию с другими обработчиками ответов. Также он более читабелен, чем операторы if-elseif-else

Похожие вопросы