aspose file tools*
The moose likes Swing / AWT / SWT and the fly likes a challenge; contest if you will Big Moose Saloon
  Search | Java FAQ | Recent Topics | Flagged Topics | Hot Topics | Zero Replies
Register / Login
JavaRanch » Java Forums » Java » Swing / AWT / SWT
Bookmark "a challenge; contest if you will" Watch "a challenge; contest if you will" New topic
Author

a challenge; contest if you will

Randall Twede
Ranch Hand

Joined: Oct 21, 2000
Posts: 4340
    
    2

a lot of you think the best way to handle action events is a separate anonymous inner class for each button. i challenge this. since java 7 i can have a switch statement that uses Strings. i think my code is shorter, more readable and more maintainable. i also contend it is just as object oriented. as for the ButtonListener class or the actionPerformed() method being too long, it is only two local variables and a switch statement.

so here is the challenge. i will post my code. if someone wants to rewrite it to use anonymous inner classes, we can let JavaRanch decide which they prefer.



SCJP
Visit my download page
Rob Spoor
Sheriff

Joined: Oct 27, 2005
Posts: 19670
    
  18

I definitely wouldn't use anonymous inner classes for each buttons here. A lot of the buttons share functionality, so I'd definitely use that. A quick untested example:
Unfortunately, 1/x and sqrt are the only ones remaining that actually share code, although you'd need an abstract method for that.

Some hints about your existing code:
1) Change isNegative = isNegative ? false : true; into isNegative = !isNegative;
2) If you decide to stick with the large switch statement, you can group "+", "-", "*", "%" and "/" like this:


SCJP 1.4 - SCJP 6 - SCWCD 5 - OCEEJBD 6
How To Ask Questions How To Answer Questions
Randall Twede
Ranch Hand

Joined: Oct 21, 2000
Posts: 4340
    
    2

i take that as 1/0 in favor of my code.
thanks rob for the help optimizing my code.
i changed it because i thought the binary() method and the operator variable should be in the inner class not the outer class.
i will also use Rob's changes. i will post the new when i can get online with my laptop. im using the state's computer now. the challenge still stands.
Matthew Brown
Bartender

Joined: Apr 06, 2010
Posts: 4363
    
    8

Randall Twede wrote:i take that as 1/0 in favor of my code.
thanks rob for the help optimizing my code.
i changed it because i thought the binary() method and the operator variable should be in the inner class not the outer class.
i will also use Rob's changes. i will post the new when i can get online with my laptop. im using the state's computer now. the challenge still stands.

How do you interpret Rob posting an alternative that doesn't have all the different responses in a single event handler - that, crucially, follows the important principle of cohesion (unlike your code - which is why I'd say it's not as object-oriented) - as 1/0 in your favour?
Randall Twede
Ranch Hand

Joined: Oct 21, 2000
Posts: 4340
    
    2

it sounded like he agreed.

i'll rewrite it taking rob's observations in mind

Paul Clapham
Bartender

Joined: Oct 14, 2005
Posts: 18541
    
    8

Randall Twede wrote:it sounded like he agreed.


Yes, it did. And yet if you look at the code that Rob posted after he "agreed", there are two ActionListeners there.

You're making this into a contest between "15 ActionListeners" and "1 ActionListener". In my opinion (like what Rob posted) neither is the winner. The right answer is a number around 3 or 4.
Matthew Brown
Bartender

Joined: Apr 06, 2010
Posts: 4363
    
    8

Paul Clapham wrote:You're making this into a contest between "15 ActionListeners" and "1 ActionListener". In my opinion (like what Rob posted) neither is the winner. The right answer is a number around 3 or 4.

3 or 4 ActionListener classes, but there would be about 15 instances in Rob's version.

The point I'd come back to is that ActionListeners, like any other class, should be cohesive. They should do one thing. A single class handling 10 events that differ only in the number they print is great - just parameterise the class. I don't think anyone is arguing for separate anonymous listeners in that case, as you'd have needless repetion (Don't Repeat Yourself). I think the original challenge is misleading.
Randall Twede
Ranch Hand

Joined: Oct 21, 2000
Posts: 4340
    
    2

cohesion...i fail to see how my code is not cohesive. perhaps you could point it out to me. i really should read code people post. Rob's code addresses the one thing i didn't like about mine. the repeated code "if (!text.equals(""))"

ActionListeners, like any other class, should be cohesive. They should do one thing.

i argue that it does do one thing. it handles the action events

i might change it to two event handlers though to get rid of the repeated if's

the only other "problem" i see is, from a MVC point of view, i am mixing the model, view, and controller in my event handler. however it is a very small program.
 
 
subject: a challenge; contest if you will