英文:
How to reduce cyclomatic complexity of this java method
问题
if (user.getUserId() != null && user.getEmail() != null
&& user.getName() != null) {
return 1;
} else if (user.getUserId() != null && user.getEmail() != null) {
return 2;
} else if (user.getUserId() != null && user.getName() != null) {
return 3;
} else if (user.getUserId() != null) {
return 4;
} else if (user.getEmail() != null && user.getName() != null) {
return 5;
} else if (user.getName() != null) {
return 6;
} else if (user.getEmail() != null) {
return 7;
} else {
return 0;
}
class Users {
String userId;
String email;
String name;
// getters and setters...
}
英文:
I have the next java implementation, but it has a SonarQube issue and i don't know how to reduce the method.
if (user.getUserId() != null && user.getEmail() != null
&& user.getName() != null) {
return 1;
} else if (user.getUserId() != null && user.getEmail() != null) {
return 2;
} else if (user.getUserId() != null && user.getName() != null) {
return 3;
} else if (user.getUserId() != null) {
return 4;
} else if (user.getEmail() != null && user.getName() != null) {
return 5;
} else if (user.getName() != null) {
return 6;
} else if (user.getEmail() != null) {
return 7;
} else {
return 0;
}
when the user is a Pojo of Users
class Users {
String userId;
String email;
String name;
//getters and setters...
}
答案1
得分: 3
关于"责任链模式"的内容。
public abstract class Handler {
protected Handler next;
public abstract Integer process(User user);
}
Handler checkIdEmailName = new Handler() {
@Override
public Integer process(User user) {
boolean check = false;
if (check) {
return 1;
} else {
return next.process(user);
}
}
};
Handler checkIdEmail = new Handler() {
@Override
public Integer process(User user) {
boolean check = false;
if (check) {
return 1;
} else {
return next.process(user);
}
}
};
checkIdEmailName.next = checkIdEmail;
Integer result = checkIdEmailName.process(new User());
英文:
what about Pattern of Chain of Responsibility.
public abstract class Handler {
protected Handler next;
public abstract Integer process(User user);
}
Handler checkIdEmailName = new Handler() {
@Override
public Integer process(User user) {
boolean check = false;
if (check) {
return 1;
}else {
return next.process(user);
}
}
};
Handler checkIdEmail = new Handler() {
@Override
public Integer process(User user) {
boolean check = false;
if (check) {
return 1;
}else {
return next.process(user);
}
}
};
checkIdEmailName.next = checkIdEmail;
Integer result = checkIdEmailName.process(new User());
答案2
得分: 0
将其分解为其他方法将减少该方法本身的复杂性。类似于:
if (user.getUserId() != null && user.getEmail() != null
&& user.getName() != null) {
return 1;
} else if (user.getUserId() != null && user.getEmail() != null) {
return 2;
} else if (user.getUserId() != null && user.getName() != null) {
return 3;
} else if (user.getUserId() != null) {
return 4;
} else {
return yourNewMethod();
}
private int yourNewMethod(User user) {
if (user.getEmail() != null && user.getName() != null) {
return 5;
} else if (user.getName() != null) {
return 6;
} else if (user.getEmail() != null) {
return 7;
} else {
return 0;
}
}
英文:
Break it to another methods would reduce the complexity of the method itself.
Something like
if (user.getUserId() != null && user.getEmail() != null
&& user.getName() != null) {
return 1;
} else if (user.getUserId() != null && user.getEmail() != null) {
return 2;
} else if (user.getUserId() != null && user.getName() != null) {
return 3;
} else if (user.getUserId() != null) {
return 4;
} else {
return yourNewMethod();
}
private int yourNewMethod(User user) {
if (user.getEmail() != null && user.getName() != null) {
return 5;
} else if (user.getName() != null) {
return 6;
} else if (user.getEmail() != null) {
return 7;
} else {
return 0;
}
}
答案3
得分: 0
首先,我们应该强调认知复杂性的含义,以及如何降低它才会有意义。主要是通过提取彼此相关的事物来实现。
当审视您的方法中的前四个条件时,它们有一个相似之处,那就是对 userId 的检查。这意味着您可以将下面的检查轻松合并为一个分支。这将更容易理解,还为提取提供了良好的方式
if (user.getUserId() != null) {
if (user.getEmail() != null
&& user.getName() != null) {
return 1;
} else if (user.getEmail() != null) {
return 2;
} else if (user.getName() != null) {
return 3;
} else {
return 4;
}
} else if (user.getEmail() != null && user.getName() != null) {
return 5;
} else if (user.getName() != null) {
return 6;
} else if (user.getEmail() != null) {
return 7;
} else {
return 0;
}
基于这一点,您可以进一步轻松提取,例如
if (user.getUserId() != null) {
return fancyNameForAdditionalUserIdChecks(user);
} else if (user.getEmail() != null && user.getName() != null) {
return 5;
} else if (user.getName() != null) {
return 6;
} else if (user.getEmail() != null) {
return 7;
} else {
return 0;
}
private int fancyNameForAdditionalUserIdChecks(Users user) {
if (user.getEmail() != null
&& user.getName() != null) {
return 1;
} else if (user.getEmail() != null) {
return 2;
} else if (user.getName() != null) {
return 3;
} else {
return 4;
}
}
现在,第一个 if 中只有四个主要分支 - 不关心 userid 为 null 的开发人员可以简单地跳过它,他将不需要处理所有这些分支。而如果需要处理,他只需要检查一个条件,其余部分更容易理解。
在处理这种情况时,您始终应该考虑某人如何阅读代码。他会获得什么好处,以及您可以如何减少认知负担,以便后续的开发人员能够更容易理解。机器永远不会读不懂我们的代码,但编写易于人类阅读的代码才是难点。
附注:
我更愿意重新设计您的 Dao 方法,在这种情况下,将该逻辑放入 dao 中,通过向 dao 提供参数映射来处理此逻辑,如 dao.find(Map<String, Object> params)
,您可以基于可用或不可用的字段来处理这一逻辑。
英文:
so first of all we should emphasis what cognitive complexity is all about, and how it would make sense to reduce it. Mainly by extracting things which belongs together.
When taking a look at your method with the first 4 conditions, they share a similarity, and it is the check for userId. This means you can easily combine the checks below into one branch. Which will be easier to grasp, and also offers a nice way for extraction
if (user.getUserId() != null) {
if (user.getEmail() != null
&& user.getName() != null) {
return 1;
} else if (user.getEmail() != null) {
return 2;
} else if (user.getName() != null) {
return 3;
} else {
return 4;
}
} else if (user.getEmail() != null && user.getName() != null) {
return 5;
} else if (user.getName() != null) {
return 6;
} else if (user.getEmail() != null) {
return 7;
} else {
return 0;
}
Based on this you can further extract easily like
if (user.getUserId() != null) {
return fancyNameForAdditionalUserIdChecks(user);
} else if (user.getEmail() != null && user.getName() != null) {
return 5;
} else if (user.getName() != null) {
return 6;
} else if (user.getEmail() != null) {
return 7;
} else {
return 0;
}
private int fancyNameForAdditionalUserIdChecks(Users user) {
if (user.getEmail() != null
&& user.getName() != null) {
return 1;
} else if (user.getEmail() != null) {
return 2;
} else if (user.getName() != null) {
return 3;
} else {
return 4;
}
}
Now we only have four main branches in the first if - a developer who does not care about cases where the userid is null, can simple skip it, and he will not need to process all those branches. Where as if needs to, he has only one condition to first check, and the rest is easier to grasp.
To handle such a case, you always should think about, how somebody is reading code. What benefit he gets, and were you can reduce the cognitive load so it will gets easier for the following developer to understand. Machines will never have problem reading our code, but writing code which is easy to read by humans, that is the hard part.
Sidenote:
i would rather rework your Dao methods, and in this case put that logic into the dao, by providing a map of paramaters to the dao, and let the dao handle such a logic. like dao.find(Map<String, Object> params)
and you could handle this based on the fields available or not available.
答案4
得分: 0
以下是简化后的代码:
boolean userIdAvailable = user.getUserId() != null;
boolean emailAvailable = user.getEmail() != null;
boolean nameAvailable = user.getName() != null;
short response = checkAll(userIdAvailable, emailAvailable, nameAvailable);
if (response == 0) {
response = checkEmailAndName(emailAvailable, nameAvailable);
if (response == 0) {
checkUserIdAndName(userIdAvailable, nameAvailable);
}
}
short checkAll(boolean userIdAvailable, boolean emailAvailable, boolean nameAvailable) {
return (short)(userIdAvailable && emailAvailable && nameAvailable ? 1 : 0);
}
short checkEach(boolean userIdAvailable, boolean emailAvailable, boolean nameAvailable) {
if (userIdAvailable) {
return 4;
} else if (emailAvailable) {
return 7;
} else if (nameAvailable) {
return 6;
}
return 0;
}
short checkUserIdAndEmail(boolean emailAvailable, boolean userIdAvailable) {
return (short)(userIdAvailable && emailAvailable ? 2 : 0);
}
short checkEmailAndName(boolean emailAvailable, boolean nameAvailable) {
return (short)(emailAvailable && nameAvailable ? 5 : 0);
}
short checkUserIdAndName(boolean userIdAvailable, boolean nameAvailable) {
return (short)(userIdAvailable && nameAvailable ? 3 : 0);
}
英文:
Here is the reduce complexity code,
boolean userIdAvailable = user.getUserId() != null;
boolean emailAvailable = user.getEmail() != null;
boolean nameAvailable = user.getName();
short response = checkAll(userIdAvailable, emailAvailable, nameAvailable);
if (response == 0) {
response = checkEmailAndName(emailAvailable, nameAvailable);
if (response == 0) {
checkUserIdAndName(userIdAvailable, nameAvailable);
}
}
short checkAll(boolean userIdAvailable, boolean emailAvailable, boolean nameAvailable) {
return userIdAvailable && emailAvailable && nameAvailable ? 1 : 0;
}
short checkEach(boolean userIdAvailable, boolean emailAvailable, boolean nameAvailable) {
short response = 0;
if(userIdAvailable) {
response = 4;
} else if(emailAvailable) {
response = 7;
} else if(nameAvailable) {
response = 6;
}
return response;
}
short checkUserIdAndEmail(boolean emailAvailable, boolean userIdAvailable) {
return userIdAvailable && emailAvailable ? 2 : 0
}
short checkEmailAndName(boolean emailAvailable, boolean nameAvailable) {
return emailAvailable && nameAvailable ? 5 : 0
}
short checkUserIdAndName(boolean userIdAvailable, boolean nameAvailable) {
return userIdAvailable && nameAvailable ? 3 : 0
}
通过集体智慧和协作来改善编程学习和解决问题的方式。致力于成为全球开发者共同参与的知识库,让每个人都能够通过互相帮助和分享经验来进步。
评论