Как избежать многого, если иначе условия

Ответ 1

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

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) {
        eventType = xpp.next();
        if (xpp.getText()!=null){
            map.put( soapResponse, current += xpp.getText());
        }
    }
    eventType = xpp.next();
}

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

Ответ 2

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

• Создайте класс интерфейса для обработки ответов (IMyResponse)
  • Используйте этот IMyResponse для создания классов AdvisoryBoardResponse, EditorialBoardResponse
• Создайте словарь с значением soapresponse как ключом, а ваша стратегия - значением
• Затем вы можете использовать методы класса IMyResponse, получая его из словаря

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

// 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);

Ответ 3

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

Ответ 4

Кроме комментария 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();

Ответ 5

Вы не упомянули, можете ли вы использовать 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, конечно, находится в интерфейсе...

Ответ 6

Вы можете создать интерфейс 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);
}

Ответ 7

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

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

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

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

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

Ответ 8

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

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);
}

Это чистый код, не так ли?