GeeCON Prague 2014*
The moose likes Beginning Java and the fly likes Comments on my first program to use an object Big Moose Saloon
  Search | Java FAQ | Recent Topics | Flagged Topics | Hot Topics | Zero Replies
Register / Login


JavaRanch » Java Forums » Java » Beginning Java
Bookmark "Comments on my first program to use an object" Watch "Comments on my first program to use an object" New topic
Author

Comments on my first program to use an object

Pete Tyo
Ranch Hand

Joined: May 11, 2005
Posts: 38
I would appreciate any suggestions on this code below that I wrote. I have to do it for a college class, but I want to make sure that I am correctlly learning the Java language to include correct indents, comments, etc.. code reusability etc.... Fire away.. I need to learn..
Ofcourse both the Gifts.class and ProgramProject3.class are in the same directory...



Now the main program code..
Roger Chung-Wee
Ranch Hand

Joined: Sep 29, 2002
Posts: 1683
Encapsulation is one of the important things to learn about OO programming. Carefully consider whether you need to use the public modifier everywhere. Just think what can happen if some other class were to directly modify the totalCollected variable ...


SCJP 1.4, SCWCD 1.3, SCBCD 1.3
Ryan McGuire
Ranch Hand

Joined: Feb 18, 2005
Posts: 1007
    
    3
All these are just my own suggestions. If anyone disagrees with them, let's discuss.

1. You could learn javadoc.

2. Some of the indenting in the main class looks ugly, but I bet that it's because your editor makes TABs four characters wide, but they got converted to eight spaces wide when you copy-and-pasted to your message.

3. Rather than having main() throw Exception, I'd use a try/catch block.

4. totalCollected in Gifts should be private.

5. I personally hate zero-information comments. You have two of them:

//instance variables to describe object attributes
and
//Gifts(): default constructor

Unless your instructor wants them there to prove that you know what a default constructor is, I'd get rid of them.

6. The name of the Gifts variable in main() is a bit misleading. Try using the variable name and the class in this sentence: <var> is a <type>. "Student is a Gifts," doesn't sound quite right. "Student is a Person," or, "Student is a SeminarRegistrant," sound good. You might try studentGifts, or graduationMoney. This is just a style issue; what you have is technically correct.

Similarly, what does the Gifts class represent? If it's money collected for some cause, then the name of the method totalSavings() is a bit off. The money hasn't been "saved", but rather "collected" or "contributed". It's even a standard practice to name methods that simply return a value getXxx() , where xxx matches the variable. So changing that method to getTotalCollected() would make sense to me.

7. I would print out the total collected so far each time through the loop, perhaps just above the menu.

8. Since you have an "else if" that breaks out of the while loop if the input was "4", there's no need to repeat that condition at the top of the while() loop. You could change the while condition to "while (true) {" or you could get rid of the break from the else if block. If you're printing out the total before the menu and removing the break, then that else if block becomes empty. In that case, you could invert the test and use that as the test for invalid input:



9. I don't see the point of having the extra Hit Enter Key To Continue. Why not just continue without pausing?

In order of importance, I would order these... (most)8, 3, 4, 7, 9, 6, 5, 1, 2.

What do I like?
A. Your formatting is nice (with the exception of the previously mentioned TAB size).
B. You explained why main() might throw an Exception near the top of the method. Beautiful.
C. You have a comment that tells that totalCollected is in US dollars.
D. You commented closing braces that are far from the corresponding opening ones.
E. Oooh a Scanner. They're new in Java 1.5.
F. Gifts, as a class to track contributions and convert currencies, was basically well implemented and well used.

Ryan
[ May 18, 2005: Message edited by: Ryan McGuire ]
Steven Bell
Ranch Hand

Joined: Dec 29, 2004
Posts: 1071
I would just add that anytime I see more than a half a dozen lines inside a
'public static void main(String[] args)'
method I cringe and die a bit inside.

There is a thing know as the 'Inventors Pardox'. It basically states that solving the general problem is easier than solving the specific problem. It's an interesting read and can be easily looked up on google.

With that in mind I often try to ask myself what is the general problem I'm trying to solve with this section of code. On that line I would probably move the code that displays the choices and captures the input into another class that maybe takes an Array of Strings for the menu choices. Then you have a general purpose console more or less, and it would probably take less code.

I would also use BigDecimal instead of double to preserve accuracy, but that's probably not a big deal.
Ravi Srinivas
Greenhorn

Joined: Sep 04, 2003
Posts: 26
Would just like to add that declaring constants such as DollarToEuro = 1.234 at the beginning of your class might be better. Then inside the methods, you could just use amount*DollarToEuro. This way if you wish to change the exchange rate it only need be done in one place, you wont have to scroll through the whole code wondering where to make the change.
Matt Fielder
Ranch Hand

Joined: Oct 27, 2004
Posts: 158
Try this...

Java Ranch's style guide

It helped me when trying to decide on a style that I liked and was effecient for my uses. Alwasy remember though that you might be required to use a different style depending on who you're coding for.
Ryan McGuire
Ranch Hand

Joined: Feb 18, 2005
Posts: 1007
    
    3
Originally posted by Ravi Srinivas:
Would just like to add that declaring constants such as DollarToEuro = 1.234 at the beginning of your class might be better.


Ooh, good one. I can't believe I didn't pick up on that.

Originally posted by Steven Bell:
I would just add that anytime I see more than a half a dozen lines inside a
'public static void main(String[] args)'
method I cringe and die a bit inside.


I wouldn't set main() apart for this treatment. For instance, I would think it would be even worse if all he did was declare another (static?) method with all this code in it and do nothing in main() but call the new method. That would increase complexity without any real gain.

However, if you're saying some of the code in this particular main() could be sucked out and put into a displayMenu() and performAction() methods, then I basically agree. BUT... the menu and the actions have to know about each other because they have to share the input-char-to-action correspondence. If you take the menu displaying and action taking code out of main(), there isn't a whole lot left. It would be possible to set up the input-action mapping in some sort of data structure that could be passed to displayMenu() and performAction() methods, but I think we're startingto talk overkill.

This has to be considered on a case-by-case basis. (I hate how often I have to say that. But I guess that what keeps me employed. )

Ryan
miguel lisboa
Ranch Hand

Joined: Feb 08, 2004
Posts: 1281
just my two cents:
Ofcourse both the Gifts.class and ProgramProject3.class are in the same directory...

you can hardcode that:
write, as first line, on every java file from this app:


java amateur
Steven Bell
Ranch Hand

Joined: Dec 29, 2004
Posts: 1071
Originally posted by Ryan McGuire:

However, if you're saying some of the code in this particular main() could be sucked out and put into a displayMenu() and performAction() methods, then I basically agree. BUT... the menu and the actions have to know about each other because they have to share the input-char-to-action correspondence. If you take the menu displaying and action taking code out of main(), there isn't a whole lot left.

This has to be considered on a case-by-case basis. (I hate how often I have to say that. But I guess that what keeps me employed. )

Ryan


I was thinking something along these lines:
Pete Tyo
Ranch Hand

Joined: May 11, 2005
Posts: 38
Been working with the Gifts.class.. When I change the Instance Variable and Constructor variable to private... I get the below compile error when I compile the ProgramProject3.java..???



I was under the impression that if I use private variables, I can still define a public method to display the private variables data?

Thanks,

Pete
miguel lisboa
Ranch Hand

Joined: Feb 08, 2004
Posts: 1281
When I change the Instance Variable and Constructor variable to private... I get the below compile error when I compile the ProgramProject3.java..???
making a contructor private means noboby outside your class can create an object of that type

I was under the impression that if I use private variables, I can still define a public method to display the private variables data?
the common practice is to mark vars private and then supply public accessors:
 
Consider Paul's rocket mass heater.
 
subject: Comments on my first program to use an object