• Post Reply Bookmark Topic Watch Topic
  • New Topic
programming forums Java Mobile Certification Databases Caching Books Engineering Micro Controllers OS Languages Paradigms IDEs Build Tools Frameworks Application Servers Open Source This Site Careers Other Pie Elite all forums
this forum made possible by our volunteer staff, including ...
Marshals:
  • Campbell Ritchie
  • Jeanne Boyarsky
  • Ron McLeod
  • Paul Clapham
  • Liutauras Vilda
Sheriffs:
  • paul wheaton
  • Rob Spoor
  • Devaka Cooray
Saloon Keepers:
  • Stephan van Hulst
  • Tim Holloway
  • Carey Brown
  • Frits Walraven
  • Tim Moores
Bartenders:
  • Mikalai Zaikin

Comments on my first program to use an object

 
Ranch Hand
Posts: 38
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
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..
 
Ranch Hand
Posts: 1683
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
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 ...
 
Bartender
Posts: 1205
22
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
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 ]
 
Ranch Hand
Posts: 1071
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
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.
 
Greenhorn
Posts: 26
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
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.
 
Ranch Hand
Posts: 158
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
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
Bartender
Posts: 1205
22
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

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
 
Ranch Hand
Posts: 1282
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
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:
 
Steven Bell
Ranch Hand
Posts: 1071
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

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
Posts: 38
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
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
Posts: 1282
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

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:
reply
    Bookmark Topic Watch Topic
  • New Topic