具有多个 if 条件的编码标准

我需要根据某些条件在实体中设置数据。

我在下面使用了设置数据

if (StringUtils.isNotBlank(customerVO.getGender())) {
mstCustomer.setGender(customerVO.getGender());
}
if (StringUtils.isNotBlank(customerVO.getBirthDate())) {
mstCustomer.setDob(DateUtils.getUtilDate(customerVO.getBirthDate()));
}
if (StringUtils.isNotBlank(customerVO.getAdd1())) {
mstCustomer.setAddress1(customerVO.getAdd1());
}
if (StringUtils.isNotBlank(customerVO.getAdd2())) {
mstCustomer.setAddress2(customerVO.getAdd2());
}
if (StringUtils.isNotBlank(customerVO.getAdd3())) {
mstCustomer.setAddress3(customerVO.getAdd3());
}
if (StringUtils.isNotBlank(customerVO.getPincode())) {
mstCustomer.setPinCode(customerVO.getPincode());
}
if (StringUtils.isNotBlank(customerVO.getStateName())) {
MstState state = mstStateRepository.findByName(customerVO.getStateName());
mstCustomer.setMstState(state);
}
if (StringUtils.isNotBlank(customerVO.getCity())) {
MstCity city = mstCityRepository.findByName(customerVO.getCity());
mstCustomer.setMstCity(city);
}
if (StringUtils.isNotBlank(customerVO.getIdentificationType())) {
mstCustomer.setIdentificationType(customerVO.getIdentificationType());
}
if (StringUtils.isNotBlank(customerVO.getIdentificationData())) {
mstCustomer.setIdentificationData(customerVO.getIdentificationData());
}
MstStatus mstStatus = mstStatusRepository.findOne(MstStatusEnum.CUST_ACTIVE.getStatusCode());
if (mstStatus != null) {
mstCustomer.setMstStatus(mstStatus);
}
if (!StringUtils.isBlank(customerVO.getMaritalStatus())) {
mstCustomer.setMaritalStatus(customerVO.getMaritalStatus());
}
if (StringUtils.isBlank(customerVO.getWeddingAnniversary())) {
mstCustomer.setWeddingAnniversary(DateUtils.getDateFromString(customerVO.getWeddingAnniversary()));
}
if (StringUtils.isNotBlank(customerVO.getMotherTongue())) {
mstCustomer.setMotherTongue(customerVO.getMotherTongue());
}
if (StringUtils.isNotBlank(customerVO.getFamilySize())) {
mstCustomer.setFamilySize(Integer.valueOf(customerVO.getFamilySize()));
}
if (StringUtils.isNotBlank(customerVO.getAdultsSize())) {
mstCustomer.setAdultsSize(Integer.valueOf(customerVO.getAdultsSize()));
}
if (StringUtils.isNotBlank(customerVO.getNoOfKids())) {
mstCustomer.setNoOfKids(Integer.valueOf(customerVO.getNoOfKids()));
}
if (StringUtils.isNotBlank(customerVO.getChilddob1())) {
mstCustomer.setChilddob1(DateUtils.getDateFromString(customerVO.getChilddob1()));
}
if (StringUtils.isNotBlank(customerVO.getChilddob2())) {
mstCustomer.setChilddob2(DateUtils.getDateFromString(customerVO.getChilddob2()));
}
if (StringUtils.isNotBlank(customerVO.getProfession())) {
mstCustomer.setProfession(customerVO.getProfession());
}

但是声纳抛出这个异常：Refactor this method to reduce its Cognitive Complexity from 27 to the 15 allowed.

请建议重构上述代码的最佳方法是什么。

使用 lambda 似乎非常可行：

private void setIfNotBlank(String value, Consumer<String> setter)  {
setConditionally(value, setter, StringUtils::isNotBlank);
}
// if you don't need non-string arguments you can inline this method
private <T> void setConditionally(T value, Consumer<T> setter, Predicate<T> shouldSet) {
if (shouldSet.test(value)) setter.accept(value);
}

然后

if (StringUtils.isNotBlank(customerVO.getBirthDate())) {
mstCustomer.setDob(DateUtils.getUtilDate(customerVO.getBirthDate()));
}
if (StringUtils.isNotBlank(customerVO.getCity())) { 
MstCity city = mstCityRepository.findByName(customerVO.getCity()); 
mstCustomer.setMstCity(city); 
}

会成为

setIfNotBlank(customerVO.getBirthDate(), birthDate -> mstCustomer.setDob(DateUtils.getUtilDate(birthDate)));
setIfNotBlank(customerVO.getCity(), cityName -> { 
MstCity city = mstCityRepository.findByName(cityName); 
mstCustomer.setMstCity(city); 
});

这非常适合Optional。首先，创建一个帮助器方法，将每个可能为空白的字段转换为Optional<String>。

Optional<String> optional(String value) {
return StringUtils.isNotBlank(value)
? Optional.of(value)
: Optional.empty();
}

然后，像这样重写代码：

optional(customerVO.getGender())   .ifPresent(mstCustomer::setGender);
optional(customerVO.getBirthDate()).map(DateUtils::getUtilDate)
.ifPresent(mstCustomer::setDob);
optional(customerVO.getAdd1())     .ifPresent(mstCustomer::setAddress1);
optional(customerVO.getAdd2())     .ifPresent(mstCustomer::setAddress2);
optional(customerVO.getAdd3())     .ifPresent(mstCustomer::setAddress3);
optional(customerVO.getPincode())  .ifPresent(mstCustomer::setPinCode);
optional(customerVO.getStateName()).map(mstStateRepository::findByName)
.ifPresent(mstCustomer::setMstState);
optional(customerVO.getCity())     .map(mstCityRepository::findByName)
.ifPresent(mstCustomer::setMstCity);
optional(customerVO.getIdentificationType()).ifPresent(mstCustomer::setIdentificationType);
optional(customerVO.getIdentificationData()).ifPresent(mstCustomer::setIdentificationData);
Optional.of(MstStatusEnum.CUST_ACTIVE.getStatusCode())
.map(mstStatusRepository::findOne)
.ifPresent(mstCustomer::setMstStatus);
optional(customerVO.getMaritalStatus())     .ifPresent(mstCustomer::setMaritalStatus);
optional(customerVO.getWeddingAnniversary()).map(DateUtils::getDateFromString)
.ifPresent(mstCustomer::setWeddingAnniversary);
optional(customerVO.getMotherTongue()).ifPresent(mstCustomer::setMotherTongue);
optional(customerVO.getFamilySize())  .map(Integer::valueOf).ifPresent(mstCustomer::setFamilySize);
optional(customerVO.getAdultsSize())  .map(Integer::valueOf).ifPresent(mstCustomer::setAdultsSize);
optional(customerVO.getNoOfKids())    .map(Integer::valueOf).ifPresent(mstCustomer::setNoOfKids);
optional(customerVO.getChilddob1())   .map(DateUtils::getDateFromString).ifPresent(mstCustomer::setChilddob1);
optional(customerVO.getChilddob2())   .map(DateUtils::getDateFromString).ifPresent(mstCustomer::setChilddob2);
optional(customerVO.getProfession())  .ifPresent(mstCustomer::setProfession);

仅当字段为非空时，ifPresent才会调用命名函数。map有助于将值从一种类型转换为另一种类型。请注意，这有助于展平逻辑，以便映射与设置分开。

在我看来，这段代码从根本上无法改进。就其本质而言，它是乏味和重复的。它将一个对象的属性转换为另一个对象的属性，而Java根本没有提供一种简短的方式来表达它。

考虑其他答案：它们将原始代码的单个 if 语句转换为另一个语句，因此您最终会得到 20 次optional()调用或 20 次setIfNotBlank()调用，而不是 20ifs——但代码的主要问题是有 20 个长而相似的语句，所以它们没有太大改进，您仍然在一种方法中以 20 个长而相似的语句结尾。

你可以转向反思：发明一些注释并用它们注释customerVO和mstCustomer对象的类别，然后在"对于第一个对象的每个注释，在第二个对象上找到相应的注释，执行分配"的脉络中重写此方法。但是现在，有了 20 个长而外观相似的if语句，您有 40 个长且外观相似的注释，它们位于两个不同的文件中。这更糟糕，所以如果你选择这样做，试着只注释其中一个类，而不是两个类：那么你最后只会有 20 个注释，所以事情和以前一样糟糕。

您可以转向代码生成：发明一些简单的属性描述格式(或找到现有的格式)，编写一个小工具，该工具将采用此描述并为customerVO和mstCustomer对象的类生成代码。现在，而不是 20 个长而相似的if语句，而是 20 个希望简短的、可能看起来不太相似的属性描述，这些描述是用只有您知道的语言和构建链中的附加工具。

正如你所看到的，你无法逃避写20个长而无聊的相似的东西。

所以我的答案是：不要重构它。将此代码以及customerVO和mstCustomer对象的类标记为已连接，并确保始终同步更改它们。这并不意味着为它编写一些单元测试——尽管你也应该这样做——这意味着建立一个过程。这不再是一个技术问题：这是一个人的问题。告诉其他开发人员这段代码;在此代码中写注释"不要忘记检查其他部分";查看提交以及代码这些部分中的更改。您不能依靠自动化工具来保持此代码的正确性，因此您和您的同事必须记住手动执行此操作。

解决此问题的最简单方法是为所有这些作业编写 setter。

喜欢：

private void setGender(GenderObject customerVO){
if (StringUtils.isNotBlank(customerVO.getGender())) {
this.setGender(customerVO.getGender());
}
}

如果可以以任何一种方式调用 setter，则可以通过将 null 处理分解为可重用的转换方法来简化代码：

mstCustomer.setGender(parseString(customerVO.getGender());
mstCustomer.setDob(parseDate(customerVO.getDob());
...

哪里

String parseString(String s) {
return StringUtils.isBlank(s) ? null : s;
}

等等。

无论您选择哪种方法，消除冗余条件都将有利于可维护性和正确性。

我实际上是这个意思，我需要的所有证据都在你现有的代码中：

if (!StringUtils.isBlank(customerVO.getMaritalStatus())) {
mstCustomer.setMaritalStatus(customerVO.getMaritalStatus());
}
if (StringUtils.isBlank(customerVO.getWeddingAnniversary())) {
mstCustomer.setWeddingAnniversary(DateUtils.getDateFromString(customerVO.getWeddingAnniversary()));
}
if (StringUtils.isNotBlank(customerVO.getMotherTongue())) {
mstCustomer.setMotherTongue(customerVO.getMotherTongue());
}

看到不一致的条件了吗？第一种和最后一种情况是有意义的，但只有在没有指定结婚纪念日的情况下才会设置结婚纪念日！

通过复制粘贴和混合两种不同的空处理逻辑变体，引入了一个错误，并且由于没有人有足够的耐心真正阅读这么多重复，因此从未注意到。

总之，不要重复代码。即使是像空处理这样看似微不足道的事情。

相关内容

最新更新

热门标签：