File APIs for Java Developers
Manipulate DOC, XLS, PPT, PDF and many others from your application.
http://aspose.com/file-tools
Win a copy of Soft Skills: The software developer's life manual this week in the Jobs Discussion forum!
  • Post Reply
  • Bookmark Topic Watch Topic
  • New Topic

Could you give me your opinion on whether my code is okay

 
Mark Toddd
Greenhorn
Posts: 11
  • 0
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hi all.

I'd like to get some opinions on whether this is okay in terms of format and placement of code.
I wasn't sure whether i should declare my classes at the top of the code or in another position.... ?




Thanks
 
salvin francis
Bartender
Posts: 1195
8
Eclipse IDE Google Web Toolkit
  • 0
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Why are you writing your entire code in main ?
 
Mark Toddd
Greenhorn
Posts: 11
  • 0
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hi

Because I'm a noob and I don't know better
How would you suggest I write it better?

Cheers
 
salvin francis
Bartender
Posts: 1195
8
Eclipse IDE Google Web Toolkit
  • 0
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Well, java is all about reusability, portability and crap like that

The reason i asked you that question was that i needed to understand why you thought of doing so. You have written a GUI program, trust me, you are not a 'noob'

here is what you probably need to do:
Grab a cup of coffee

See, your class "FirstFrame" indicates a frame being used, here is what i would have done:




Also the ActionListener code, being simple can be tackled in this manner:



where messageClicked() is a method in MainFrame class
next, I would prefer menubar, mypanel, etc. to be instance variables instead of local variables.

try these changes and post the new code here ...
 
Ernest Friedman-Hill
author and iconoclast
Marshal
Pie
Posts: 24204
34
Chrome Eclipse IDE Mac OS X
  • 0
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
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.
 
salvin francis
Bartender
Posts: 1195
8
Eclipse IDE Google Web Toolkit
  • 0
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
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"
 
Gregg Bolinger
GenRocket Founder
Ranch Hand
Posts: 15302
6
Chrome IntelliJ IDE Mac OS X
  • 0
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
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.
 
Consider Paul's rocket mass heater.
  • Post Reply
  • Bookmark Topic Watch Topic
  • New Topic