Anonymous class syntax is certainly clunky in Java (closures can't come too soon, as far as I'm concerned), but I'm not convinced that this is a particularly compelling alternative. In addition, the author might be a little too concerned with the efficiency aspects of his code: "saves about 0.5kb per menuitem in terms of compiled code size. The classloader will only have to fetch one class and the garbage collector will have less objects to monitor" (We care about kilobytes? Really?) and "don't use equals() for comparison. It's safe and sane to use == here, as it is what Object.equals() would do anyway, so save the method call for the sake of performance." (I doubt that this is a tight inner loop where shaving those few extra nanoseconds has any great impact.) As Donald "Homeboy" Knuth said, "We should forget about small efficiencies, say about 97% of the time: premature optimization is the root of all evil."
Using "==" instead of equals() in the actionPerfomed() method is not only better performance wise (note: This code executes as a reaction to user input and therefore cannot be fast enough), it also makes the code easier to read and most important of all, it is what you actually mean at that point. You do not want the handler for a menuitem to be invoked by anything but exactly that menuitem.
As for saving 500 Bytes: Well, those might not seem like much sitting on a modern Desktop harddisk, but think about Applets and mobile devices which may not have virtually unlimited storage/bandwidth at their disposal. Here, every kilobyte might be an issue.
However, both points are only minor additional benefits, the suggested method has over the clumsy approach of using inner classes to just forward actionevents back into the main class. The real subject of the article (which you seem to have missed) is code clarity and here the suggested pattern wins hands down over writing potentially hundreds of lines of code in the constructor, doing nothing but defining anonymous forwarding classes.
Actually, I didn't miss the point of the article, which was essentially to have one listener with a whopping if-else statement. I simply didn't agree, as I said, that it was a "particularly compelling alternative" to using inner classes. I believe that both anonymous class syntax and a large if-else statement are fairly inelegant ways to wire up callbacks on a menu. I don't even think there is an elegant way to do it in Java, so that whatever approach you go for is largely a matter of personal preference until (or if) we get closures in the language.
With regard to performance, I'd be surprised indeed if a user would notice any difference in responsiveness if you used == or .equals(). (For example, the JIT could inline the method call for all I know). But don't take my word for it: measure it. We all tend to have really bad intuitions about what makes code fast or slow.
Comments
jfpoilpret replied ago:
Looks mroe like something I would not want to see in any code produced in my company. I wouldn't call it a "pattern".
MattRussell replied ago:
Anonymous class syntax is certainly clunky in Java (closures can't come too soon, as far as I'm concerned), but I'm not convinced that this is a particularly compelling alternative. In addition, the author might be a little too concerned with the efficiency aspects of his code: "saves about 0.5kb per menuitem in terms of compiled code size. The classloader will only have to fetch one class and the garbage collector will have less objects to monitor" (We care about kilobytes? Really?) and "don't use equals() for comparison. It's safe and sane to use == here, as it is what Object.equals() would do anyway, so save the method call for the sake of performance." (I doubt that this is a tight inner loop where shaving those few extra nanoseconds has any great impact.) As Donald "Homeboy" Knuth said, "We should forget about small efficiencies, say about 97% of the time: premature optimization is the root of all evil."
onyxbits replied ago:
Using "==" instead of equals() in the actionPerfomed() method is not only better performance wise (note: This code executes as a reaction to user input and therefore cannot be fast enough), it also makes the code easier to read and most important of all, it is what you actually mean at that point. You do not want the handler for a menuitem to be invoked by anything but exactly that menuitem.
As for saving 500 Bytes: Well, those might not seem like much sitting on a modern Desktop harddisk, but think about Applets and mobile devices which may not have virtually unlimited storage/bandwidth at their disposal. Here, every kilobyte might be an issue.
However, both points are only minor additional benefits, the suggested method has over the clumsy approach of using inner classes to just forward actionevents back into the main class. The real subject of the article (which you seem to have missed) is code clarity and here the suggested pattern wins hands down over writing potentially hundreds of lines of code in the constructor, doing nothing but defining anonymous forwarding classes.
MattRussell replied ago:
Actually, I didn't miss the point of the article, which was essentially to have one listener with a whopping if-else statement. I simply didn't agree, as I said, that it was a "particularly compelling alternative" to using inner classes. I believe that both anonymous class syntax and a large if-else statement are fairly inelegant ways to wire up callbacks on a menu. I don't even think there is an elegant way to do it in Java, so that whatever approach you go for is largely a matter of personal preference until (or if) we get closures in the language.
With regard to performance, I'd be surprised indeed if a user would notice any difference in responsiveness if you used == or .equals(). (For example, the JIT could inline the method call for all I know). But don't take my word for it: measure it. We all tend to have really bad intuitions about what makes code fast or slow.
Voters For This Link (5)
Voters Against This Link (2)