Code Review C++

Discussion in 'Mac Programming' started by unkn0wnsoldier, Mar 2, 2010.

  1. unkn0wnsoldier macrumors regular

    unkn0wnsoldier

    Joined:
    Jul 4, 2009
    Location:
    Fountain, Co
    #1
    Hello ladies and gents!

    This is my first post to the programming section. After reading through here, I've noticed that most of you guys are less than thrilled for someone to ask for some programming help with no input of their own, so you'll be happy to see that I've completed my own very simple code.

    A bit about me: I'm 28 and just returning back to school to hopefully become a computer engineer. I'm taking my very first programming class which is C++. Mind you, I have no formal programming experience, other than what I have learned since the beginning of this semester. I had to write a simple program that calculates how many dollars, quarters, dimes, etc, from what ever the input is (in cents).

    I would be so appreciative if someone could look this over for me and let me know if there is a more elegant way to do the same thing. We haven't gotten into creating functions yet but, I was thinking that a loop of some sort could do the trick but, can't think of any way to implement one.

    Here's the code:

    Code:
    #include <iostream>
    using namespace std;
    
    int main () 
    {
    int remainder, pennies, dollars, quarters, dimes, nickles;
    
    
    
    cout <<"Enter the number of pennies - ";
    cin >> pennies;
    cout << "\n\nYou have " << pennies << " cents which is equivalent to:\n\n";
    
    dollars = pennies / 100;
    remainder = pennies % 100;
    quarters = remainder / 25;
    remainder = remainder % 25;
    dimes = remainder / 10;
    remainder = remainder % 10;
    nickles = remainder / 5;
    remainder = remainder % 5;
    pennies = remainder;
    
    
    
    cout << dollars << " dollar(s)\n";
    
    cout << quarters << " quarter(s)\n";
    cout << dimes << " dime(s)\n";
    cout << nickles << " nickle(s), and \n";
    cout << pennies << " cent(s)\n\n";
    cout << "Thanks for running the change program!" << endl;
    
    }

    Thanks,
    Pat
     
  2. chown33 macrumors 604

    Joined:
    Aug 9, 2009
    #2
    Here's a hint...

    coin names: "dollar", "quarter", "dime", "nickel", "penny"
    cent values: 100, 25, 10, 5, 1
     
  3. unkn0wnsoldier thread starter macrumors regular

    unkn0wnsoldier

    Joined:
    Jul 4, 2009
    Location:
    Fountain, Co
    #3
    Are you saying I should assign those values respectively like:

    int dollar = 100, quarter = 25, etc
     
  4. chown33 macrumors 604

    Joined:
    Aug 9, 2009
    #4
    Do you know what an array is?

    That's a serious question, not sarcastic.

    If the answer is "No", then wait until after you know what an array is. Because without arrays, or a similar composite data structure, I don't think you can really improve on what you have. The fact that you perceive some potential for looping is a good sign, even if you don't know how to write it as code yet.

    For reference, a composite data structure is any data structure composed from smaller distinct elements. The smaller elements may be composites, or they may be primitives (types like int, char, double). Arrays are one of the simplest composites.

    http://en.wikipedia.org/wiki/Array_data_type
    http://en.wikipedia.org/wiki/Array_data_structure

    If that's too much, don't worry about. You'll get there eventually.

    On the other hand, if you do know what an array is, or can work it out from Wikipedia articles, then there are probably enough clues for you to make the code.
     
  5. Littleodie914 macrumors 68000

    Littleodie914

    Joined:
    Jun 9, 2004
    Location:
    Rochester, NY
    #5
    I think you could probably improve it with something like... (pseudo-code)

    Code:
    
    int numNickels = 0, numDimes = 0, numQuarters = 0, numDollars = 0;
    int numPennies = 427;
    
    while (numPennies >= 100) {
        numPennies -= 100;
        numDollars++;
    }
    while (numPennies >= 25) {
       numPennies -= 25;
       numQuarters++;
    }
    ...
    
    It might not be shorter line-wise, but it introduces control flow into your program, and is a second way of approaching the problem. :)
     
  6. unkn0wnsoldier thread starter macrumors regular

    unkn0wnsoldier

    Joined:
    Jul 4, 2009
    Location:
    Fountain, Co
    #6
    Thanks. I'll check this out.

    No. We haven't covered that. Thank you for looking into this for me. It does give me some reassurance in being able to implement what I've learned in a half decent way.

    Thank you,
    Pat
     
  7. lloyddean macrumors 6502a

    Joined:
    May 10, 2009
    Location:
    Des Moines, WA
    #7
    Or if familiar with pointers calling a routine along the lines of

    Code:
    int coins_of_value(int* pennies, int coin_value)
    would reduce the need for so many local variables.

    Additionally it could be used with the hint from chown33
     
  8. unkn0wnsoldier thread starter macrumors regular

    unkn0wnsoldier

    Joined:
    Jul 4, 2009
    Location:
    Fountain, Co
    #8
    I want to thank everyone for their input. It seems, however, that my basic knowledge of C++ and, programming in general, is not sufficient to grasp the concepts that you guys are introducing. I do have some more topics and information to research thanks to your tips.

    Thank you,
    Pat
     
  9. iSee macrumors 68040

    iSee

    Joined:
    Oct 25, 2004
    #9
    I've gotta say, I disagree that using arrays, functions, or loops would improve this code.

    IMO, the next goal for code, after functioning correctly, is to be simple and concise. This code is no more complex than the problem it solves, so that's perfect.

    Other comments:

    The variable names are good and the code is well organized. It's self-documenting code (i.e., does not require additional comments to understand), which is great.

    The only thing I'd add is that it's good practice to handle any kind of input, even nonsensical or unexpected input. E.g., you could check if the user input is negative and abort in that case.

    I guess to get really picky, I wouldn't typically reuse a variable the way pennies is reused. At first it holds the total number of cents the user entered and at the end it holds the number of pennies of change. The units are the same, but the concept is different, so I'd use separate variables.

    Bottom Line: I'd have given you an A if I were grading this assignment. Good luck on your degree.
     
  10. chown33 macrumors 604

    Joined:
    Aug 9, 2009
    #10
    Again, don't worry about it. You'll get there eventually. We were all once where you are.

    Years and years ago, when I used to teach C, I would have students revisit exercises and solve them in new ways using new knowledge. I always thought this was useful.

    First, it shows students their own progress in a way nothing else can.

    Second, it gets them used to doing fixes, improvements, and maintenance on existing code, without breaking anything (as demonstrated by writing tests).

    Third, it lets them analyze their own old code, where they know their own thought processes at that time, because it's their own code, not someone else's. This gives them insight into how to write programs for other readers, such as their future selves. Suppose you could go back and tell yourself what comments to add, how to arrange data and construct functions, etc. What would you do? Now, write today's program so you won't have to go back and tell yourself those things.
     
  11. chown33 macrumors 604

    Joined:
    Aug 9, 2009
    #11
    I've been thinking about this more, and there's another approach that doesn't involve looping, but does involve a function. I'll outline it without giving the code, so you can revisit it and solve it with a function. It can be done with or without an array and a loop.

    First, look at these code fragments:
    Code:
    ...
    quarters = remainder / 25;
    remainder = remainder % 25;
    dimes = remainder / 10;
    remainder = remainder % 10;
    nickles = remainder / 5;
    remainder = remainder % 5;
    ...
    cout << quarters << " quarter(s)\n";
    cout << dimes << " dime(s)\n";
    cout << nickles << " nickle(s), and \n";
    
    There's a clear repeating symmetry here:
    Code:
    [I][COLOR="Green"]count[/COLOR][/I] = remainder / [I][COLOR="Green"]multiplier[/COLOR][/I];
    remainder = remainder % [I][COLOR="Green"]multiplier[/COLOR][/I];
    ..
    cout << [I][COLOR="Green"]count[/COLOR][/I] << " [I][COLOR="Green"]denomination[/COLOR][/I](s)\n";
    
    
    Also notice that dollars can be calculated the same way, simply by starting with remainder set to the total amount. It will also work for pennies, where the multiplier is 1.

    What we have is the outline for a function. It has one parameter for the current remainder (total remaining cents), another for the multiplier (how many cents in that denomination), and another parameter for the name of the denomination (a string). The count is printed immediately, and the remainder is returned for further use, to determine the next smaller coin count.

    For bonus points, make it work with Canadian coinage, which includes $0.50, $1 (loonie), and $2 (toonie) denominations.
    http://en.wikipedia.org/wiki/Coins_of_the_Canadian_dollar
     
  12. gnasher729 macrumors P6

    gnasher729

    Joined:
    Nov 25, 2005
    #12
    So what happens with 100 pennies?
     
  13. Sander macrumors 6502

    Joined:
    Apr 24, 2008
    #13
    My two cents (oh! the pun!): I'd grade you an A as well.

    I don't think arrays would be useful here.

    I do agree with Chown33 you should revisit this code later, after you've been introduced to functions and references, and break out the "repeating patterns" into a separate function.

    Have fun learning to program!
     
  14. gnasher729 macrumors P6

    gnasher729

    Joined:
    Nov 25, 2005
    #14
    Nothing wrong with your code. You might want to see what your program does if I enter -1049 and fix whatever problems it has. For a challenge, try to output a string like

    "103 pennies = One dollar and 3 pennies."
    "230 pennies = 2 dollars, one quarter, and one nickle."
     
  15. Littleodie914 macrumors 68000

    Littleodie914

    Joined:
    Jun 9, 2004
    Location:
    Rochester, NY
    #15
    Snap. *facepalm* :eek: Updated above.
     

Share This Page