Have you ever received a warning about Cyclomatic Complexity while working with a class in Android Studio? Would you like to know how to fix it? If yes, keep reading…
What is cyclomatic complexity?
Here is a definition from Wikipedia:
“The cyclomatic complexity of a section of source code is the number of linearly independent paths within it. For instance, if the source code contained no control flow statements (conditionals or decision points), such as IF statements, the complexity would be 1, since there is only a single path through the code.” — Wikipedia
Ok, so that particular definition is a tad verbose, but it’s essentially saying that the more control structures (if, else, switch, etc.) you have in your code the more you introduce complexity into your program.
Why is high cyclomatic complexity bad?
Short answer, it can make it more difficult to write unit tests that cover all possible branches of a given function as well as hinder debugging efforts. Others argue that it affects the readability of your code, making it more challenging for another developer to understand the intent of your functions and/or classes.
How do you fix it?
You start by reducing the number of paths through any given function. Let’s take a look at a very contrived example.
public void overlyComplexMethod(Video video) {
if (video != null && video.getStreamUrl() != null) {
switch (video.getCategory()) {
case "CAT1" :
playVideo(video);
if (video.getLargeImageUrl() == null) {
video.setLargeImageUrl("http://www.largeImage.png");
}
updateMetadata(video);
break;
case "CAT2" :
if (video.getLargeImageUrl() == null) {
video.setLargeImageUrl("http://www.smallImage.png");
}
updateMetadata(video);
break;
case "CAT3" :
if (video.getLargeImageUrl() == null) {
video.setLargeImageUrl("http://www.mediumImage.png");
}
updateMetadata(video);
break;
default:
break;
}
}
}
This function has several different paths through it, notice the use of a switch statement with several cases and within the cases more if statements. The cyclomatic complexity of this method is 9, ideally you would want to have most functions with a value of 8 or less. So let’s clean it up!
First we’re going to move the switch statement into its own method.
public void overlyComplexMethod(Video video) {
if (video != null && video.getStreamUrl() != null) {
updateVideoBasedOnCategory(video);
}
}
private void updateVideoBasedOnCategory(Video video) {
switch (video.getCategory()) {
case "CAT1" :
playVideo(video);
if (video.getLargeImageUrl() == null) {
video.setLargeImageUrl("http://www.largeImage.png");
}
updateMetadata(video);
break;
case "CAT2" :
if (video.getLargeImageUrl() == null) {
video.setLargeImageUrl("http://www.smallImage.png");
}
updateMetadata(video);
break;
case "CAT3" :
if (video.getLargeImageUrl() == null) {
video.setLargeImageUrl("http://www.mediumImage.png");
}
updateMetadata(video);
break;
default:
break;
}
}
By making this simple change we’ve already reduced the complexity down to a value of 7. The next step would be to look for code duplication among the case statements and then create one method that they can all share. Let’s see how that might look…
public void overlyComplexMethod(Video video) {
if (video != null && video.getStreamUrl() != null) {
updateVideoBasedOnCategory(video);
}
}
private void updateVideoBasedOnCategory(Video video) {
switch (video.getCategory()) {
case "CAT1" :
playVideo(video);
updateVideoMetaDataAndUrl(video, "http://www.largeImage.png");
break;
case "CAT2" :
updateVideoMetaDataAndUrl(video, "http://www.smallImage.png");
break;
case "CAT3" :
updateVideoMetaDataAndUrl(video, "http://www.mediumImage.png");
break;
default:
break;
}
}
private void updateVideoMetaDataAndUrl(Video video, String url) {
video.setLargeImageUrl(url);
updateMetadata(video);
}
Now by extracting out this common method, updateVideoMetaDataAndUrl, we have reduced the cyclomatic complexity to 4. We could reduce this further by eliminating the need for a switch statement with polymorphism. I leave that to you as an exercise.
Conclusion
As we can see it’s a simple refactoring effort to reduce the cyclomatic complexity of methods and by extension classes. I would however like to mention that having unit tests in place before you begin the refactoring is ideal to ensure you don’t make any breaking changes.
If you want to learn more about writing clean code, I highly recommend the book Clean Code by Robert Martin.
