This week's book giveaway is in the OCMJEA forum.
We're giving away four copies of OCM Java EE 6 Enterprise Architect Exam Guide and have Paul Allen & Joseph Bambara on-line!
See this thread for details.
The moose likes Beginning Java and the fly likes Just finished my first java program :-) Can you please give me suggestions on making my code better? Big Moose Saloon
  Search | Java FAQ | Recent Topics | Flagged Topics | Hot Topics | Zero Replies
Register / Login


Win a copy of OCM Java EE 6 Enterprise Architect Exam Guide this week in the OCMJEA forum!
JavaRanch » Java Forums » Java » Beginning Java
Bookmark "Just finished my first java program :-) Can you please give me suggestions on making my code better?" Watch "Just finished my first java program :-) Can you please give me suggestions on making my code better?" New topic
Author

Just finished my first java program :-) Can you please give me suggestions on making my code better?

Sebastion Hill
Ranch Hand

Joined: Jul 10, 2009
Posts: 48
Hey all, I am pretty excited because I just finished my first program. It's pretty basic but it is the first part of an on going personal project. It runs good and does what I was hoping it would but now I am curious as to which parts I could have done better on.

Just as a background note, I plan on making an application for my android phone. I typically wish that I could have a better planner (cause I am not that great at time management) so I thought I would make an application suited to my needs. This will be based upon the philosophy from guy who wrote 7 successful habits etc., Stephen Covey I think.

Anyways, this is the first part and I plan on making a GUI for this program after I get feedback on ways to improve the code. I broke the arrays into 4 different sections here because I plan on making them color coded in my GUI (e.g. blue for highest priority, maize for second highest, green third, and maybe red for the lowest priority).

But yeah, if anyone wouldn't mind providing feedback on ways that I can improve I would be EXTREMELY grateful. :-) My code is listed below:



and the activity class ...

David Newton
Author
Rancher

Joined: Sep 29, 2008
Posts: 12617

I'm not sure I would have made separate arrays for the priorities--most likely I would have simply made the priority a part of the object. It also seems to be recursive, which I'm pretty sure isn't your intent (for example, selectionScreen() calls prompt() which calls selectionScreen() etc. It'll take awhile, but eventually you'll run out of memory and crash.

The checkActivityArray() method seems to basically repeat large chunks of code. Any time you find yourself using "paste" as a programming paradigm it's time to re-think and re-factor.
Seetharaman Venkatasamy
Ranch Hand

Joined: Jan 28, 2008
Posts: 5575

1.Use LogWriter instead of System.out.println()


2.

Check method parameter before using them
Harshit Rastogi
Ranch Hand

Joined: Apr 15, 2008
Posts: 131


Change this to


you should always use the interface class . Thats a good practice. Because if in case you want to change it to Vector simply change the implementation


<a href="http://technologiquepanorama.wordpress.com" target="_blank" rel="nofollow">My Techie Blog</a><br /><a href="http://www.java-questions.com" target="_blank" rel="nofollow">Java Questions</a>
Sebastion Hill
Ranch Hand

Joined: Jul 10, 2009
Posts: 48
David Newton wrote:I'm not sure I would have made separate arrays for the priorities--most likely I would have simply made the priority a part of the object. It also seems to be recursive, which I'm pretty sure isn't your intent (for example, selectionScreen() calls prompt() which calls selectionScreen() etc. It'll take awhile, but eventually you'll run out of memory and crash.

The checkActivityArray() method seems to basically repeat large chunks of code. Any time you find yourself using "paste" as a programming paradigm it's time to re-think and re-factor.


Thanks David. I originally thought about trying to make an array of arraylists for the priority parts but I didn't know if that was possible. I like your ideal to put the priority into the actual object, I think I'll take some time and think about how I can do that (still a newbie but it sounds good). Thanks again, I'll remember the "pasting code" you mentioned for future effors
Sebastion Hill
Ranch Hand

Joined: Jul 10, 2009
Posts: 48
seetharaman venkatasamy wrote:1.Use LogWriter instead of System.out.println()


2.

Check method parameter before using them


Hi Seetharaman, thanks for the feedback. I am curious as to the benefits of using LogWriter instead of System.out.println() ... the reason why I ask is that I've never heard of LogWriter and wanted to double check to make sure it's not personal preferences

Also, by check method parameter did you mean just add a try-catch or is there something else I should be doing?
Sebastion Hill
Ranch Hand

Joined: Jul 10, 2009
Posts: 48
Harshit Rastogi wrote:

Change this to


you should always use the interface class . Thats a good practice. Because if in case you want to change it to Vector simply change the implementation


Hey Harshit, for my clarification ... I just need to change the first ArrayList to List ... due to the interface class? I'll read up on it more to get a better understanding but if you don't mind explaining it that would be great, because I am not really sure about your vector comment that follows. Thanks again
Rob Spoor
Sheriff

Joined: Oct 27, 2005
Posts: 19685
    
  20

Suppose you find that Priority4 gives you problems. You debug a bit, and find it is a threading issue - two threads are adding elements at the same time, and the ArrayList breaks. You think: I should synchronize the ArrayList! Big problem: now you have to change all points of access to the ArrayList. Auch!

Now, let's say you declared the field as List. Now all you need to do is change just one line of code:
Or, a method I prefer:
And voila! All access to Priority4 is now synchronized*.


* When using an Iterator you should still synchronize manually:
Still, much less code needs to be changed.


SCJP 1.4 - SCJP 6 - SCWCD 5 - OCEEJBD 6
How To Ask Questions How To Answer Questions
Fred Hamilton
Ranch Hand

Joined: May 13, 2009
Posts: 679
I'm gonna nitpick on something that doesn't really affect functionality, but may affect readability.

Give some thoughts to the names you give your Boolean variables and methods. IMO they should be more suggestive of yes or no answers or true/false answers

So instead of things like Boolean importance and Boolean urgency, you would have things like Boolean isImportant and Boolean isUrgent.

if( activity1.isImportant ) sounds better to me than if( activity1.importance ) , see what I mean?
Andrew Ceronoski
Greenhorn

Joined: Apr 21, 2009
Posts: 16
Adding a GUI is really nice...but also a lot of work. For letting the user know they made an error try using a dialog box (real easy ).

Another idea is to use HTML to format your text. You often print multiple lines....you could use one "System.out.println" and use the html command \n to have a new line.

Neat program!
Campbell Ritchie
Sheriff

Joined: Oct 13, 2005
Posts: 38519
    
  23
I would only suggest using html tags in the documentation comments, which are supposed to be translated to html anyway. There are some Swing components which support html too.

The \n character is not an html tag, and since Java5 it has been far better to use the %n tag which obtains the platform-specific line separator than \n.
Sebastion Hill
Ranch Hand

Joined: Jul 10, 2009
Posts: 48
Rob Prime wrote:Suppose you find that Priority4 gives you problems. You debug a bit, and find it is a threading issue - two threads are adding elements at the same time, and the ArrayList breaks. You think: I should synchronize the ArrayList! Big problem: now you have to change all points of access to the ArrayList. Auch!

Now, let's say you declared the field as List. Now all you need to do is change just one line of code:
Or, a method I prefer:
And voila! All access to Priority4 is now synchronized*.


* When using an Iterator you should still synchronize manually:
Still, much less code needs to be changed.


Thanks Rob. I will take you up on your suggestion and I will also research synchronized more to get a better understanding. I know what synchronized means in real world talk but a lot of these comments are going over my head but I am looking forward to understanding them all better! :-) thanks again
Sebastion Hill
Ranch Hand

Joined: Jul 10, 2009
Posts: 48
Fred Hamilton wrote:I'm gonna nitpick on something that doesn't really affect functionality, but may affect readability.

Give some thoughts to the names you give your Boolean variables and methods. IMO they should be more suggestive of yes or no answers or true/false answers

So instead of things like Boolean importance and Boolean urgency, you would have things like Boolean isImportant and Boolean isUrgent.

if( activity1.isImportant ) sounds better to me than if( activity1.importance ) , see what I mean?


No worries, I was being lazy and I am glad you pointed this out. I need to make sure to develop good habits ;-)
Sebastion Hill
Ranch Hand

Joined: Jul 10, 2009
Posts: 48
Campbell Ritchie wrote:I would only suggest using html tags in the documentation comments, which are supposed to be translated to html anyway. There are some Swing components which support html too.

The \n character is not an html tag, and since Java5 it has been far better to use the %n tag which obtains the platform-specific line separator than \n.


Andrew and Campbell, html is kind of over my head right now, though I plan on looking into it. Do I need to know HTML before learning Java or is it something that I can learn afterwards? Is html needed for the GUI?
Campbell Ritchie
Sheriff

Joined: Oct 13, 2005
Posts: 38519
    
  23
The only bit of Java which usually uses HTML is the documentation comments. You don't need HTML to write Java. You can write documentation comments without knowing any HTML. You can create 99.99% of any GUIs without knowing HTML.
If you wish to execute an applet you will need a 3-line HTML document, which you can more or less copy out of the books.

There are a few GUI components where you get a better-looking text display with HTML. Your documentation comments are converted into HTML, which looks better if you put a few tags in. It will take ages to learn the amount of HTML you would actually use for Java programming, at a rough estimate maybe half an hour .
Sebastion Hill
Ranch Hand

Joined: Jul 10, 2009
Posts: 48
Campbell Ritchie wrote:The only bit of Java which usually uses HTML is the documentation comments. You don't need HTML to write Java. You can write documentation comments without knowing any HTML. You can create 99.99% of any GUIs without knowing HTML.
If you wish to execute an applet you will need a 3-line HTML document, which you can more or less copy out of the books.

There are a few GUI components where you get a better-looking text display with HTML. Your documentation comments are converted into HTML, which looks better if you put a few tags in. It will take ages to learn the amount of HTML you would actually use for Java programming, at a rough estimate maybe half an hour .


Lol, I think that is a fair trade-off then. Thanks Campbell
Campbell Ritchie
Sheriff

Joined: Oct 13, 2005
Posts: 38519
    
  23
You're welcome

And for the % tags, try this Java™ Tutorials section, which is brief but easy to understand, or the Formatter class, which is comprehensive but difficult to understand. Or there might be something in your book.
Sebastion Hill
Ranch Hand

Joined: Jul 10, 2009
Posts: 48
So I have done some updates, mainly trying to use the suggestions I have received from forum members. I felt like an idiot after thinking about how I could eliminate the repeat code for the arrays ... lol, it finally hit me that I only needed 1 array (so this eliminated the need for synchronization, though I still plan on learning more about it).

Also, I updated my boolean expressions to make them better in terms of syntax and I also spent some time learning tags using System.out.format (again, VERY useful though I still have lots to learn).

I also added an automatic save feature as well as the option to delete activities (since they are now being saved and will start to add up).

I think I have still failed in regards to the recursive issues, any suggestions on fixing that? :-/

Well here it is, the updated code ...



Sebastion Hill
Ranch Hand

Joined: Jul 10, 2009
Posts: 48
If no one sees any major problems with this code then my GUI work will start on Monday :-)
 
I agree. Here's the link: http://aspose.com/file-tools
 
subject: Just finished my first java program :-) Can you please give me suggestions on making my code better?