This week's book giveaway is in the OO, Patterns, UML and Refactoring forum. We're giving away four copies of Refactoring for Software Design Smells: Managing Technical Debt and have Girish Suryanarayana, Ganesh Samarthyam & Tushar Sharma on-line! See this thread for details.
Salvin: your suggested changes add two new essentially empty methods, and make the one long method substantially longer by folding the named listener classes into the code. So I'd say your changes make the program nontrivially worse, and in no way better.
Mark: What would I do, specifically, to improve the code we're looking at?
There are some repeated calls (setVisible(), setSize()); you can remove the duplicates.
You want to defer calling setVisible() until the whole GUI is assembled (i.e., do it last).
You could break related chunks of code out into sensible-named methods; i.e., I see about six lines that could be moved into a method named setUpMenuBar(), and ten that could become setUpContentPanel() .
Many people recommend only importing the classes you use, rather than using "*" to import a whole package.
Declaring the nested classes at the top of the class is fine. I often put them at the end; either way is probably better than randomly distributing them.
Ernest Friedman-Hill wrote:So I'd say your changes make the program nontrivially worse, and in no way better.
I havent even written the program, just suggested changes as a start.
Maybe you could have pointed points you agree with me and points you do not agree with me [preferably with reasons] thus giving our friend Mark sman a better idea about java.
1. Do you agree that he should not have written his entire program in main ?
2. Do you agree with the init() method ?
3. My approach was to separate listener logic with the code using methods such as messageClicked and maybe closeFrame using anonymous class to do the calling
this, in my opinion helps when the program has a change in logic, programmer has to just change the implementation of the method itself.
I do not fully understand your point of making "one long method substantially longer"
Another thing to note is that UI code must execute on the Event Dispatching Thread (EDT). The main() will get started in a special thread the VM kicks off which is NOT on the EDT. So we need should make sure that it happens on the EDT by using the SwingUtilties.invokeLater() method like so...
Also note that it is becoming standard practice to not inherit from JFrame since so little is generally done with the JFrame itself you're better off not coupling your main class to it. But for learning purposes it's not that big a deal.