Menu
I have the following method:
private void setClientAdditionalInfo(Map map, Client client, User user) {
Map additionalInfo = (Map) map.get("additionalInfo");
if (checkMapProperty(additionalInfo, "gender")) {
client.setGender(additionalInfo.get("gender").toString());
}
if (checkMapProperty(additionalInfo, "race")) {
client.setRace(additionalInfo.get("race").toString());
}
if (checkMapProperty(additionalInfo, "ethnicity")) {
client.setEthnicity(additionalInfo.get("ethnicity").toString());
}
.....
12 more if statements are used in the similar way. The only difference being a different setter method name and a different parameter. Now, as the same pattern is repeated again and again, is there a way to reduce the code complexity?
709
Reduce the Number of Decision Structures You might consider this one a no-brainer. If the decision structures—especially if-else and switch case—are what cause more branches in the code, it stands to reason that you should reduce them if you want to keep cyclomatic complexity at bay.
Cyclomatic complexity of a code section is the quantitative measure of the number of linearly independent paths in it. It is a software metric used to indicate the complexity of a program. It is computed using the Control Flow Graph of the program.
Avoid use of switch/case statements in your code. Use Factory or Strategy design patterns instead. Complexity of 8 (1 for each CASE and 1 for method itself). Refactoring using Factory design pattern and complexity changed to 1.
Consequences: A high cyclomatic complexity for a particular function means that the function will be difficult to understand, and more difficult to test. That in turn means functions with a cyclomatic complexity above 10 or 15 can be reasonably expected to have more undetected defects than simpler functions.
Not easily, and not without using reflection.
Using reflection you could loop through the list of values and call the appropriate method in the client object. That would get rid of the complexity and be cleaner/more robust. However it would also perform slower.
Fundamentally though you have the case where you are doing nearly but not quite the same operation over and over, that's always tricky.
102
You can do this with Java 8 functional interfaces. It'll at least get rid of the repeated conditional statements.
private void doRepetitiveThing(Map info, String key, Consumer<String> setterFunction) {
if(checkMapProperty(info, key)) {
setterFunction.accept(info.get(key).toString());
}
}
private void setClientAdditionalInfo(Map map, Client client, User user) {
Map additionalInfo = (Map) map.get("additionalInfo");
doRepetitiveThing(additionalInfo, "gender", client::setGender);
doRepetitiveThing(additionalInfo, "race", client::setRace);
doRepetitiveThing(additionalInfo, "ethnicity", client::setEthnicity);
.....
Not sure if this actually reduces the cyclomatic complexity, but it makes the code prettier. This is easier with Java 8.
private void setClientAdditionalInfo(Map<String, Object> map, Client client, User user) {
Map<String, ?> additionalInfo = (Map<String, Object>) map.get("additionalInfo");
setIfPresent(additionalInfo, "gender", client::setGender);
setIfPresent(additionalInfo, "race", client::setRace);
setIfPresent(additionalInfo, "ethnicity", client::setEthnicity);
}
private void <T> setIfPresent(Map<String, ?> additionalInfo,
String property,
Consumer<T> consumer) {
if (checkMapProperty(additionalInfo, property)) {
consumer.apply((T)additionalInfo.get(property));
}
}
If you wanted, you could make a Map<String, Consumer<?>> to avoid the repeated calls.
31
If you love us? You can donate to us via Paypal or buy me a coffee so we can maintain and grow! Thank you!
Donate Us With
