Pounds, Shillings, Pence Help!

[Sasquadge]

Hey guys I'm having a minor issue here, I'm having an issue with my sterling.cpp in the add functions sterling add(sterling s1, sterling s2) my question is how do I get it to realize when my shillings is > 20 to goto pounds and when my pence is > 12 to goto shillings? I have the driver.cpp to test out situations to make sure it all works. When it adds the numbers up it isn't just right. Any adivce?

//Driver.cpp my test
#include <iostream>
#include "sterling.h"
using namespace std;

int main(){

    sterling s;
    read(&s);
    print(s);

    sterling u = make_sterling(1, 20, 12);
    print(u);

    sterling    a = add(s, u);
    print(a);


    return 0;
}



//sterling.cpp my functions
#include <iostream>
#include "sterling.h"
using namespace std;

sterling make_sterling(int po, int s, int pe){

    sterling    temp;

    temp.pounds = po;
    temp.shillings = s;
    temp.pence = pe;

    return temp;
}

sterling make_sterling(int pe){

    sterling temp;

    temp.pounds = pe / 240;
    pe %= 240;

    temp.shillings = pe / 20;
    temp.pence = pe % 12;

    return temp;
}

sterling add(sterling s1, sterling s2){
    int i1 = s1.pounds * 240 + s1.shillings * 12 + s1.pence;
    int i2 = s1.pounds * 240 + s1.shillings * 12 + s1.pence;


    return make_sterling(i1 + i2);
}

void print(const sterling& a_sterling){

    cout.fill('0');
    cout << (char)156 << a_sterling.pounds << "." << a_sterling.shillings
        << "." << a_sterling.pence << endl;
    cout.fill(' ');
}

void read(sterling* s){

    cout <<"Please enter the pounds: ";
    cin >> s->pounds;
    cout <<"Please enter the shillings: ";
    cin >> s->shillings;
    cout <<"Please enter the pence: ";
    cin >> s->pence;
}



//header sterling.h
struct sterling
{
    int pounds;
    int shillings;
    int pence;

};

sterling make_sterling(int po, int s, int pe);
sterling make_sterling(int pe);
sterling add(sterling s1, sterling s2);
void print(const sterling& a_sterling);
void read(sterling* s);

[Sasquadge]

any help out there?

[TrustyTony]

Your code seems to run OK, not that output would make much sense for me...

[Sasquadge]

Well whats going on is when it adds the pounds.shillings.pence its not making 12+pence into shillings and 20+ shillings into pounds if that makes sense. The output I have in my driver is just the user enters there own pounds shillings and pence then it adds to the one stored in the program and shows the total.

[Sasquadge]

There is something in one of these functions that is making my output wrong. I;m just not seeing it any ideas?

sterling make_sterling(int pe){

    sterling temp;

    temp.pounds = pe / 240;
    pe %= 240;

    temp.shillings = pe / 20;
    temp.pence = pe % 12;

    return temp;
}

sterling add(sterling s1, sterling s2){
    int i1 = s1.pounds * 240 + s1.shillings * 12 + s1.pence;
    int i2 = s2.pounds * 240 + s2.shillings * 12 + s2.pence;


    return make_sterling(i1 + i2);
}

[TrustyTony]

You have posted many functions, but no indication how you have tested each. Actually I think you should have converted everything to pence on input and only pretty print output using the other units.

[Sasquadge]

I've tested each function but my equations are just not coming out right.
I just feel so lost at this point lol. I've had it add 1 pound 20 shillings 12 pence + 1 pound 20 shillings 12 pence = its coming out as 4 pound 1 shillings 0 pence that's not right? Also I would like it to convert the shillings and pence over when above the 20 shillings and 12 pence when it out puts the number entered. If that makes sense.

[Bob]

Double-check whether this is doing what you expect it to, keeping in mind how many pennies there are in a shilling.

temp.shillings = pe / 20;
temp.pence = pe % 12;

[Sasquadge]

should I do the remainder of both? and not divide.

    temp.shillings = pe % 19;
    temp.pence = pe % 11;

[Bob]

Footnote: the problem you're seeing is a good example of the type of problem that arises from using magic numbers in your code rather than named constants. If you're not sure what I mean, feel free to ask.

[Sasquadge]

I'm not sure what you mean if you could explain I would appreciate it.

[Bob]

should I do the remainder of both? and not divide.

No, just double check what your expression is for. You're calculating how many shillings there are in a given number of pennies, but you're diving by the number of shillings in a pound (20) rather than how many pennies in a shilling (12) -- even though you correctly use 12 on the next line.

[Sasquadge]

    temp.shillings = pe / 12;
    temp.pence = pe % 12;

[Bob]

temp.shillings = pe / 12;
temp.pence = pe % 12;

Yeah, does that help? How is your output?

Now imagine that instead of typing 12 or 20 or 240 everywhere within your code, you replaced those literals with a named constant, for example:

const int PENNIES_IN_POUNDS = 240;
const int SHILLINGS_IN_POUNDS = 20;
const int PENNIES_IN_SHILLINGS = 12;

The lines above then become:

temp.shillings = pe / PENNIES_IN_SHILLINGS;
temp.pence = pe % PENNIES_IN_SHILLINGS;

If you go one step further and use a more meaningful name rather than the abbreviation 'pe', you have something like:

temp.shillings = pennies / PENNIES_IN_SHILLINGS;
temp.pence = pennies % PENNIES_IN_SHILLINGS;

You've replaced the magic numbers (12 in this case) with constants with meaningful names (PENNIES_IN_SHILLINGS).

[Sasquadge]

The output came out correctly thank you for the help on that one it makes a lot more since now about replacing the magic numbers.