Win a copy of Cross-Platform Desktop Applications: Using Node, Electron, and NW.js this week in the JavaScript forum!
  • Post Reply Bookmark Topic Watch Topic
  • New Topic

Improve McCabe Cyclomatic Complexity within an action listener  RSS feed

 
Carlos Davila
Greenhorn
Posts: 5
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hello,

I am trying to improve the complexity but so for the only way i know how to work an action is to incorporate a bunch of If{} Else If{} statements. This is a big incremented for the cyclomatic complexity. Does anyone have any other thoughts to executing code based on a certain button that may be pressed?


Pseudo code: I have the implementation below for about 16 buttons in my program. Looking to improve. Thanks!

 
Les Morgan
Rancher
Posts: 767
19
C++ Java MySQL Database Netbeans IDE Oracle Tomcat Server
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Since the getSource() returns an object, you could make a method in your button class that makes the appropriate call--so you could do this in your event handler:
 
Carlos Davila
Greenhorn
Posts: 5
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hi Les,

Thank you for responding. I am trying to see if there is a more efficient way in matching the "e.source" with different types of actual sources, other than using about 10 else if statements
 
Les Morgan
Rancher
Posts: 767
19
C++ Java MySQL Database Netbeans IDE Oracle Tomcat Server
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Yep, I understand; you cannot get any simpler than a 1 liner.

Carlos Davila wrote:Hi Les,

Thank you for responding. I am trying to see if there is a more efficient way in matching the "e.source" with different types of actual sources, other than using about 10 else if statements
 
Carlos Davila
Greenhorn
Posts: 5
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Yes O(1) linear is the simplest.

I am looking to improve the cyclomatic complexity which racks up points when there are a lot of branches (in my case, else if statements). Are there any other typical practices people use to implement action listeners?
 
Les Morgan
Rancher
Posts: 767
19
C++ Java MySQL Database Netbeans IDE Oracle Tomcat Server
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
What you are asking for in not standard though, you are asking how to simplify the standard... this will do it.

In your button class you would have to make a method that would make the specific all you need done... that means you would need to override the myMethod() each time you add a button--so it will make the appropriate call from your call in the event handler according to your e.getSource().myMethod().
 
Les Morgan
Rancher
Posts: 767
19
C++ Java MySQL Database Netbeans IDE Oracle Tomcat Server
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
You could also do anonymous event handlers for each button, and each could be hardcoded to make the appropriate call.

Carlos Davila wrote:Yes O(1) linear is the simplest.

I am looking to improve the cyclomatic complexity which racks up points when there are a lot of branches (in my case, else if statements). Are there any other typical practices people use to implement action listeners?
 
Junilu Lacar
Sheriff
Posts: 10948
158
Android Debian Eclipse IDE IntelliJ IDE Java Linux Mac Spring Ubuntu
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Les Morgan wrote:

I don't think this will even compile as is since getSource returns an Object. You'd have to cast it to an interface or base class first then call the method.

Another option would be to use a Map<Button, Handler> where handler is again an interface or base class. If you're using Java 8, handler can be a functional interface which then opens up the opportunity to use lambdas or method references. This way, you'd simply pass e.getSource() to map.get() to lookup the appropriate handler. Then you just invoke whatever handler you get back. The nice thing about this too is that you can adhere to the Open-Closed Principle by injecting the Map.
 
Junilu Lacar
Sheriff
Posts: 10948
158
Android Debian Eclipse IDE IntelliJ IDE Java Linux Mac Spring Ubuntu
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I am assuming, of course, that e is an EventObject
 
Les Morgan
Rancher
Posts: 767
19
C++ Java MySQL Database Netbeans IDE Oracle Tomcat Server
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Junilu,
You were right I had to cast first:

Junilu Lacar wrote:
Les Morgan wrote:

I don't think this will even compile as is since getSource returns an Object. You'd have to cast it to an interface or base class first then call the method.

Another option would be to use a Map<Button, Handler> where handler is again an interface or base class. If you're using Java 8, handler can be a functional interface which then opens up the opportunity to use lambdas or method references. This way, you'd simply pass e.getSource() to map.get() to lookup the appropriate handler. Then you just invoke whatever handler you get back. The nice thing about this too is that you can adhere to the Open-Closed Principle by injecting the Map.
 
Campbell Ritchie
Sheriff
Posts: 55333
157
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I shall move you to our GUIs forum.

I agree with whoever said to use an anonymous class; of course now Java8 is here you can use a λ instead, since action listener has only one method. Write a method to correspond to each button needing an action.Your methods do not actually have to use the evt parameter. It may be better to use Action objects than action listeners.
 
Campbell Ritchie
Sheriff
Posts: 55333
157
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Carlos Davila wrote:Yes O(1) linear is the simplest. . . .
That is not called linear complexity, but constant complexity.
 
  • Post Reply Bookmark Topic Watch Topic
  • New Topic
Boost this thread!