How to reduce cyclomatic complexity inside an onClick() method

Question

Let's imagine a simple construction:

public void onClick(View view) {
    switch (view.getId()) {
        case R.id.btn_first: {
            // some code
        }
            break;
        case R.id.btn_second: {
            // some code
        }
            break;
        case R.id.btn_third: {
            // some code
        }
            break;
        // and so on
    }
}

This is a trivial piece of code that handles clicks of different buttons. But as the number of buttons grows - the cyclomatic complexity of the switch block grows with it. Is there another way to represent this code construction to reduce cyclomatic complexity of the onClick() method? Thanks in advance.

option 1: Use one click handler per button. option 2: group like buttons into click handlers and use a few click handlers with short switch statements. option 3: delegate the click handling to the view (see the Ryan Stewart answer below). — DwB, Feb 01 '13 at 14:08

score 2 · Answer 1 · answered Feb 01 '13 at 14:04

public void onClick(View view) {
    for (ClickHandler handler : allHandlers()) {
        if (handler.supports(view.getId())) {
            handler.onClick(view);
        }
    }
}

interface ClickHandler {
    boolean supports(int viewId);
    void onClick(View view);
}

Implement a ClickHandler for each branch and have allHandlers() build a list of them.

score 1 · Answer 2 · edited Nov 18 '20 at 22:53

This was one of the issues I ran into with earlier versions of GWT (Google Web Toolkit). GWT was google's answer to giving JAVA developers a way to write JavaScript code using JAVA.

We started out using a single anonymous inner class to handle each event/element on a web page. It worked fine when you didn't have very many elements, which wasn't realistic for a web page. So, as the number of elements grew it got to be unmanageable and cluttered. Then we quickly got into the same issue you are raising. Google solved it by using an event bus in later versions of GWT.

How did that solve it? They created a pattern where you abstract the conditions up one level to the event bus and let it call the proper handler.

However, getting back to your javascript question, here is a good, simple explanation for the pattern, Handling events for many elements

Really, in my thinking, what you want to do is avoid the cyclomatic complexity pattern altogether like I said by abstracting up one level with a event bus or eventhandler or map, that ultimately calls a function to avoid code duplication and solve this issue with another pattern.

I would rather create more small functions and hence make the code more component-like than allow the if/else pattern to keep reproducing itself. I've found that there is no end to the pattern once you head down that path and you will keep being forced to add more if/else statements until you refactor the code with a better solution.

I recall seeing this same thing in JAVA code where they were training COBOL developers to learn JAVA and the ones that kept their procedural mindset constantly fell into this pattern. I once saw a switch statement with 26 conditions checking for a single type condition.

Here are some good tips for Cyclomatic Complexity Refactoring Tips with good code examples.

score 0 · Answer 3 · answered Feb 01 '13 at 14:00

0

It is possible (hashmap with executors or something for example), but in my opinion it will make the code quality worse. A low cyclomatic complexity is good for high quality code, but the ultimate goal should still by high quality and maintainable code, not low cyclomatic complexity.

answered Feb 01 '13 at 14:00

Janoz

953
4
9

This code will become low quality and non-maintainable. That's the problem here. – Ryan Stewart Feb 01 '13 at 14:06

How to reduce cyclomatic complexity inside an onClick() method

3 Answers3