0

我发现了一些出色的代码，其中包含 30 多个参数给一个方法（我记错了）。该实现包含 500 多行带有if/then/else和switch块的行。

如何以干净的方式重构它？您对此有何建议？

很多实现都遍布整个应用程序，并且都推送这些参数。

问题方法：

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

7 回答 7

3

首先，我会进一步检查它的实际用法。

如果您发现使用这些参数的特定子集重复调用它，您可能可以执行以下操作：

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

如果这不合适，正如其他人建议的那样，我会创建一个SearchParams类。所有属性都将初始化为默认值，其用法仅涉及设置每次调用所需的相关搜索词，例如：

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

后者肯定会至少清理调用代码，对现有底层机制的影响最小。

于 2013-08-01T09:27:13.130 回答
2

您可以创建一个Predicate接口（或使用guava's）：

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

并将您的方法更改为：

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);
于 2013-08-01T08:31:47.450 回答
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();
于 2013-08-01T10:08:04.667 回答
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) {
  ....
}

或者

将所有参数放在地图中，并以定义明确的常量作为键

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

...

并接受地图作为参数

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

就if/else、switch而言，必须将它们划分为逻辑上的小子方法，以降低循环复杂度

于 2013-08-01T08:30:41.923 回答
1

您最好创建一个具有所有这些参数作为属性的对象，并在整个代码中处理它。

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

并仅填写您在每种情况下需要的属性。我想其中一些在某些情况下是可选的。

于 2013-08-01T08:32:16.930 回答
0

我会用一个小型 sql 数据库替换它，每个用户都有一个简单的主键。

于 2013-08-01T08:28:47.943 回答
0

用自我解释的二传手制作建设者？我知道 30 个 setter 并没有好多少，但您至少可以通过编程方式生成它们。真的有那么多必要的参数吗？

于 2013-08-01T08:32:48.533 回答