2013-08-01 2 views
0

Я нашел отличный код с более чем 30 параметрами, предоставленными методу (я потерял счет). Реализация содержит более 500 строк с , если/then/else и переключатель блок.Необходима помощь в рефакторе - метод со многими параметрами

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

Множество реализаций реализовано по всему приложению, и все эти параметры задают.

метод Проблема:

public static User findUser (
    String userCode, String lastName, String firstName, 
    String alternativeLastName, String sex, int day, int month, int year, 
    String locationParameter, String locationInfo, 
    Id groupId, Id organizationId, Id orderId, 
    Id orderGroupId, Id orderOrganizationId, 
    List<Id> groupIds, List<Id> organizationIds, 
    List<Id> orderIds, List<Id> orderGroupIds, 
    List<Id> orderOrganizationIds, 
    String timeRange, Long daysAgo, 
    Date dateFrom, Date dateUntil, 
    CodingMap codingMap, List<String> languageList, String visitType, 
    Account account, List<Group> accountGroups, 
    Organization accountOrganization, Profile profile, 
    AccessProfile accessProfile, Locale locale, int maxResults, 
    long newTimeRange, long minimumTimeRange, 
    boolean hideUserWithoutName, boolean withOrderInfo, boolean withVisitInfo, 
    boolean orderEntryAvailable, 
    boolean hideDiscontinuedResults, boolean checkPatientAccessRights, boolean searchForCopies, 
    boolean inOrderEntry, boolean allPatientCodes, 
    boolean addPatientCharacteristics, boolean showSpeciesColumn, 
    boolean patientDefinedWithPrivacyLevel, 
    boolean excludePatientsWithoutCodeInSelectedCodingSystem 
    ) throws CompanyException { 
... 
} 

Используется повсюду, как это:

User.findUser(member.getCode(), context.lastName, context.firstName, 
    context.alternativeLastName, context.sex, 
    context.birthDate.getDay(), context.birthDate.getMonth(), 
    context.birthDate.getYear(), 
    context.locationParameter, context.locationInfo, 
    null, null, null, null, null, null, null, null, null, null, 
    session.timeRange(), session.daysAgo(), session.dateFrom(), 
    session.dateUntil(), 
    order.codingMap(), order.languageList(), member.visitType(), 
    null, null, null, null, null, locale, 25, 1000L,200L, 
    session.point.booleanValue(), session.infobooleanValue(), false, 
    true, true, true, true, true, true, true, false, false, false); 
+3

O ... M ... G ...... – assylias

+1

Мой совет будет ** ** RUN. –

+1

рефакторинг? Я бы назвал это «переписыванием» кода: D – Juvanis

ответ

3

Прежде всего, я рассмотрю его фактическое использование немного дальше.

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

User.FindUserByABC(A, B, C) 
User.FindUserByXYZ(X, Y, Z) 

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

SearchParams params = new SearchParams(){A = "...", Z = "..."} 
User.FindUser(params) 

Последнее, конечно, очистить код вызова по крайней мере, с минимальным воздействием на существующих основных механизмов.

0

Я бы заменить это с небольшой базы данных SQL с помощью простого первичного ключа для каждого пользователя.

1

инкапсулировать все эти параметры в классе

class MethodParam { 
     String userCode; String lastName; String firstName; 
     String alternativeLastName; String sex; int day; int month; int year; 
     String locationParameter; String locationInfo; 
     Id groupId; Id organizationId; Id orderId; 
     Id orderGroupId; Id orderOrganizationId; 
     List<Id> groupIds; List<Id> organizationIds; 
     List<Id> orderIds; List<Id> orderGroupIds; 
     List<Id> orderOrganizationIds; 
     String timeRange; Long daysAgo; 
     Date dateFrom; Date dateUntil; 
     CodingMap codingMap; List<String> languageList; String visitType; 
     Account account; List<Group> accountGroups; 
     Organization accountOrganization; Profile profile; 
     AccessProfile accessProfile; Locale locale; int maxResults; 
     long newTimeRange; long minimumTimeRange; 
     boolean hideUserWithoutName; boolean withOrderInfo; boolean withVisitInfo; 
     boolean orderEntryAvailable; 
     boolean hideDiscontinuedResults; boolean checkPatientAccessRights; boolean searchForCopies; 
     boolean inOrderEntry; boolean allPatientCodes; 
     boolean addPatientCharacteristics; boolean showSpeciesColumn; 
     boolean patientDefinedWithPrivacyLevel; 
     boolean excludePatientsWithoutCodeInSelectedCodingSystem; 
} 

, а затем отправить этот класс методу

findUser(MethodParam param) { 
    .... 
} 

ИЛИ

положить все Params на карте с хорошо определенными константами в качестве ключей

Map<String,Object> paramMap = new HashMap<>(); 
paramMap.put(Constants.USER_CODE, userCode); 

...

и принимает карту в качестве параметра

findUser(Map<String, Object> param) { 
     .... 
} 

Насколько если/другое, переключатель, то они должны быть разделены на логические методы небольших подгрупп, чтобы уменьшить сложность cyclometric

2

Вы можете создать интерфейс Predicate (или использовать guava's):

interface Predicate<T> { boolean apply(T input); } 

в d изменить метод:

User findUser(Predicate<User> p) { 
    //find user 
} 

Вы тогда называть это так:

findUser(new Predicate<User>() { 
    public boolean apply(User user) { 
     return member.getCode().equals(user.getCode()) 
       && user.lastname.equals(context.lastname); //etc. 
    } 
}); 

Следующим шагом может стать введение UserQuery с использованием свободно синтаксис легко создавать эти запросы. Это будет выглядеть так:

UserQuery query = new UserQuery(); 
query.lastname(context.lastname) 
     .code(member.getCode()); //etc 
Predicate<User> p = query.build(); 
User user = findUser(p); 
1

Вы бы лучше создать объект со всеми этими параметрами, как atributes и иметь дело со всем этим над кодом.

public class MyObeject{ 
    String userCode, String lastName, String firstName, 
    String alternativeLastName, String sex, int day, int month, int year, 
    String locationParameter, String locationInfo, 
    Id groupId, Id organizationId, Id orderId, 
    Id orderGroupId, Id orderOrganizationId, 
    List<Id> groupIds, List<Id> organizationIds, 
    List<Id> orderIds, List<Id> orderGroupIds, 
    List<Id> orderOrganizationIds, 
    String timeRange, Long daysAgo, 
    Date dateFrom, Date dateUntil, 
    CodingMap codingMap, List<String> languageList, String visitType, 
    Account account, List<Group> accountGroups, 
    Organization accountOrganization, Profile profile, 
    AccessProfile accessProfile, Locale locale, int maxResults, 
    long newTimeRange, long minimumTimeRange, 
    boolean hideUserWithoutName, boolean withOrderInfo, boolean withVisitInfo, 
    boolean orderEntryAvailable, 
    boolean hideDiscontinuedResults, boolean checkPatientAccessRights, boolean searchForCopies, 
    boolean inOrderEntry, boolean allPatientCodes, 
    boolean addPatientCharacteristics, boolean showSpeciesColumn, 
    boolean patientDefinedWithPrivacyLevel, 
    boolean excludePatientsWithoutCodeInSelectedCodingSystem 
} 

findUser(MyObject obj); 

И заполните только те атрибуты, которые вам нужны в каждом случае. Я думаю, некоторые из них будут необязательными в некоторых случаях.

0

Сделать строитель с самоочевидными сеттерами?Я понимаю, что 30 сеттеров не намного лучше, но вы можете хотя бы генерировать их программно. И действительно ли это, что многие нужны параметров?

2

Создать метод класса с строителем для всех необходимых параметров:

public class FindUser { 
    // fields that represent necessary parameters 
    private final String lastName; 
    ... 

    // fields that represent optional parameters 
    private Id groupId; 
    ... 

    // builder class for necessary parameters 
    public static class Builder { 
     private String lastName; 
     ... 

     public Builder lastName(String lastName) { 
      this.lastName = lastName; 
      return this; 
     } 
     ... 

     public FindUser build { 
      return new FindUser(this); 
     } 
    } 

    // constructor taking a builder with all necessary parameters 
    private FindUser(Builder builder){ 
     // check here, whether all fields are really set in the builder 
     this.lastName = builder.lastName; 
     ... 
    } 

    // setters for all optional parameters 
    public FindUser groupId(Id groupId) { 
     this.groupId = groupId; 
     return this; 
    } 
    ... 

    // the action 
    public User compute() { 
     ... 
    } 
} 

Скопируйте бывшее тело метод внутри нового метода вычислительного класса метода. После этого вы можете реорганизовать этот метод, извлекая много кусков кода в свои собственные методы и, возможно, в классы.

Последние два шага:

1) Внутри старого метода findUser, создать новый объект класса метод и вызвать метод вычислений. Это не уменьшит размер списка параметров.

2) Измените все способы использования метода findUser для использования нового класса методов, а затем удалите старый метод findUser.

Использование будет:

FindUser fu = new FindUser.Builder() 
     .lastname("last name") 
     ... 
     .build() 
     .groupId(new GroupId()) 
     ...; 
User user = fu.compute();