Как провести рефакторинг этого кода?

Question

Пожалуйста, предложите при рефакторинге этого кода. Избегайте дублирования кода, множественные If's

public FormDataDTO getDataForFieldFormCHrzntalField(Field field) {
        FormDataDTO formDataDTO = new FormDataDTO();
        CHrzntalField cHrzntalField = (CHrzntalField) field;
        for (int j = 0; j < cHrzntalField.getFieldCount(); j++) {
            Field sField = cHrzntalField.getField(j);
            if (sField instanceof LabelField) {
                LabelField labelField = sField;
                String fieldName = labelField.getText();
                System.out.println("The Label field name is " + fieldName);
                formDataDTO.setFieldName(fieldName);
            } else if (sField instanceof CTextFieldBorder) {
                CTextFieldBorder cTextFieldBorder = (CTextFieldBorder) sField;
                Field ssField = cTextFieldBorder.getField(0);
                if (ssField instanceof TextField) {
                    TextField textField = ssField;
                    System.out.println("Inside TextField---- "
                            + textField.getText());
                    formDataDTO.setFieldType("TextField");
                    formDataDTO.setSelectedValue(textField.getText());
                } else if (ssField instanceof DateField) {
                    DateField dateField = ssField;
                    String dateString = dateField.toString();
                    System.out.println("dateString " + dateString);
                    formDataDTO.setFieldType("Date");
                    formDataDTO.setSelectedValue(dateString);
                }
            } else if (sField instanceof CChoiceField) {
                CChoiceField cChoiceField = (CChoiceField) sField;
                int i = cChoiceField.getSelectedIndex();
                String selectedValue = cChoiceField.getChoice(i);
                System.out.println("Choice " + selectedValue);
                formDataDTO.setFieldType("Combo");
                formDataDTO.setSelectedValue(selectedValue);
            } else if (sField instanceof CheckboxField) {
                CheckboxField checkboxField = (CheckboxField) sField;
                boolean checkStatus = checkboxField.getChecked();
                System.out.println("Check box field " + checkStatus);
                formDataDTO.setFieldType("Checkbox");
                String status = new Boolean(checkStatus).toString();
                formDataDTO.setSelectedValue(status);
            }
        }
        return formDataDTO;
    }

Answer 1

Первым шагом является создание модульного теста, проверяющего поведение этого метода. Во-вторых, «Скажи, не спрашивай» - это принцип хорошей ОО-разработки, поэтому было бы хорошо, если бы вы могли реорганизовать тип поля и его подклассы, чтобы реализовать метод, позволяющий им устанавливать необходимую информацию в FormDataDTO.

Answer 2

Вы могли бы начать, потянув каждый блок case (код внутри блоков if / else if) в свои собственные методы. Я не вижу много повторений, просто пытаюсь сделать слишком много одним способом.

Answer 3

Вы можете применить шаблон стратегии по внешнему виду;

• создать интерфейс с методами, которые вы вызываете на всех полях, скажем FieldHandler
• инициализировать карту из ClassName в FieldHandler, содержащую реализации для каждого типа поля, которое необходимо охватить (например, LabelFieldHandler, DateFieldHandler и т. Д.)
• в вашей функции doXXX вместо того, чтобы использовать instanceOf для выполнения вариаций для каждого типа поля, найдите соответствующий обработчик на вашей карте и делегируйте вызов обработчику.

псевдокод:

field = getField(j);
handler = handlerMap.get(field.className);
if (null == handler) {
  // error unknown field type
} else {
  handler.setFormData(field, formDataDTO);
}

Answer 4

Вам необходимо отправить по типу поля. Есть несколько способов сделать это:

1. Используйте операторы if, которые явно проверяют класс.

2. Сделайте так, чтобы все поля реализовывали интерфейс, реализуйте этот интерфейс соответствующим образом для каждого типа поля, а затем вызовите интерфейс.

3. Используйте карту, чтобы найти соответствующее действие для класса.

Вариант 1 - это то, что вы делаете сейчас; 2 - это то, что упоминает Стробоскоп; 3 называется паттерном стратегии по rsp. 1, как видите, немного беспорядок. 2 соединяет работу вышеописанного метода с полями, а 3 - нет. Какой из них (2 или 3) выбрать, зависит от вашего конкретного случая. Преимущество (2) состоит в том, что вы не забываете писать код для каждого нового поля (потому что вы получите ошибку компилятора, если забудете). Преимущество (3) состоит в том, что если вы хотите делать подобные вещи много раз, поля могут быть загромождены. Кроме того, (2) требуется, чтобы у вас был доступ к коду полей.

Стоит отметить, что если вы использовали Scala, а не Java, некоторые из проблем с (2) были бы устранены с чертами (и что он также имеет более хороший синтаксис для (1) с сопоставлением с образцом).

лично я бы предпочел (2), если возможно - возможно, реализовать это с делегированием. Единственное реальное преимущество (3) перед (1) состоит в том, что код более аккуратен - и есть немного дополнительной безопасности типов.

Answer 5

Добавить новый абстрактный метод в поле

public class Field {
    public abstract void updateFormData(FormDataDTO formDataDTO);
}

, а затем реализует его в каждом подклассе поля.

Наконец, ваш код становится:

public FormDataDTO getDataForFieldFormCHrzntalField(Field field) {
    FormDataDTO formDataDTO = new FormDataDTO();
    CHrzntalField cHrzntalField = (CHrzntalField) field;
    for (int j = 0; j < cHrzntalField.getFieldCount(); j++) {
            Field sField = cHrzntalField.getField(j);
            sField.updateFormData(formDataDTO);
    }
    return formDataDTO;
}

Answer 6

Вы должны использовать перегрузку методов, чтобы избежать вызовов instanceof. Каждый if (sField instanceof ...) должен быть перемещен в отдельный метод, принимая в качестве параметра нужный тип.