Arc Ranges
Arc Ranges

Reputation: 37

overloading arithmetic operators c++

I just started learning classes on C++ and I'm having quite some issues dealing with overloading arithmetic operators. First of all, in my header file I have:

#ifndef MONEY_H
#define MONEY_H
#include <iostream>
using namespace std;

class Money{
    public:
        Money(int dollars, int cents);
        Money(int dollars);
        Money();
        int getDollars() const {return dollars;};
        int getCents() const {return cents;};
        void setDollarsAndCents(int dollars, int cents);
        double getAmount() const {return amount ;};
        void setAmount(double amount);

        // Define operator functions  for comparison operators
        friend bool operator==(const Money& firstAmount, const Money& secondAmount);
        friend bool operator<(const Money& firstAmount, const Money& secondAmount);
        friend bool operator>(const Money& firstAmount, const Money& secondAmount);

        //Define operator functions for arithmetic operators
        friend Money operator+(const Money& firstAmount, const Money& secondAmount);
        friend Money operator-(const Money& firstAmount, const Money& secondAmount);
        friend Money operator*(const Money& money, int n);
        friend Money operator/(const Money& money, int n);

        //Define the output and input operator
        friend ostream& operator<<(ostream& outStream, const Money& money);
    private:
        int dollars, cents;
        double amount;
};
#endif

then I implemented the operator+ on an implementation file, Money.cpp

  #include "Money.h"

// Construct a money object with dollars and cents
Money::Money(int newDollars, int newCents)
{
    dollars = newDollars;
    cents = newCents;
}
// Construct a money object with JUST the dollars
Money::Money(int newDollars)
{
    dollars = newDollars;
    cents = 0;
}
// Construct a money object with no arguments (default amount = 0)
Money::Money()
{
    amount = 0.0;
}
// Set dollars and cents
void Money::setDollarsAndCents(int newDollars, int newCents)
{
    dollars = newDollars;
    cents = newCents;
}
// Set monetary amount
void Money::setAmount(double newAmount)
{
    //convert cents automatically if >= 100
    newAmount = dollars + cents/100.0;
    amount = newAmount;
}
// Test if two Money objects are equal or not
bool operator==(const Money& firstAmount, const Money& secondAmount)
{   
    return (firstAmount.amount == secondAmount.amount);
}
// Test if the first operand is less than the second operand
bool operator<(const Money& firstAmount, const Money& secondAmount)
{
    return (firstAmount.amount < secondAmount.amount);
}
// Test if the first operand is greater than the second operand
bool operator>(const Money& firstAmount, const Money& secondAmount) 
{
    return (firstAmount.amount > secondAmount.amount);
}
// Add two Money objects
Money operator+(const Money& firstAmount, const Money& secondAmount)
{
    //assume cents < 100
    int carry = 0;
    int finalCents = firstAmount.cents + secondAmount.cents;

    if (finalCents >= 100){
        carry += 1;
        finalCents -= 100;
    }
    int finalDollars = firstAmount.dollars + secondAmount.dollars + carry;

    return Money(finalDollars, finalCents);
}
// Subtract two Money objects
Money operator-(const Money& firstAmount, const Money& secondAmount)
{
    int borrow = 0;
    int finalCents = firstAmount.cents - secondAmount.cents;
    if (finalCents < 0){
        finalCents += 100;
        borrow = 1;
    }
    int finalDollars = firstAmount.dollars - secondAmount.dollars - borrow;
    return Money(finalDollars, finalCents);
}
// Multiply two Money objects
Money operator*(const Money& money, int n)
{
    return money.amount * n;
}
// Divide two Money objects
Money operator/(const Money& money, int n)
{
    int quotient = money.amount / n;
    // check if there isn't a remainder
    if ( quotient * n == 0)
        return money.amount / n;
    else // there's a remainder
        return money.dollars / n + money.cents / (n * 100);
}
// Define the output operator
ostream& operator<<(ostream& outputStream, const Money& money)
{
    outputStream << money.amount;
   return outputStream;
}

Lastly, in the main method on my TestMoney.cpp, I have:

#include "Money.h"
using namespace std;

int main()
{
    Money m1(-35),m2(53, 35);

    //Test operator == (false)
    cout << "m1 == m2 = " << (m1 == m2 ? "true" : "false")  << endl;

    Money m3(-35),m4(35); 

    //Test operator < (true)
    cout << "m3 < m4 = " << (m3 < m4 ? "true" : "false")  << endl;

    Money m5(-35),m6(53, 35); 

    //Test operator > (false)
    cout << "m5 > 6 = " << (m5 > m6 ? "true" : "false")  << endl;

    Money m7(12,50),m8(25,55); 
    // $12.50 & $25.50 = $38.05
    //Test operator +
    cout << "m7 + m8 = $" << (m7 + m8) << endl;

    //~ Money m9(5,75), m10(100); 
    //~ // $5.75 - $100 = $-94.25
    //~ //Test operator -
    //~ cout << "m9 - m10 = $" << m9 - m10 << endl;

    //~ Money m11(25,75);
    //~ int n = 5;
    //~ // $25.75 * $5 = $128.75
    //~ //Test operator *
    //~ cout << "m11 * m12 = $" << m11 * n << endl;

    //~ Money m13(115,75);
    //~ n = 3;
    //~ // $115.75 / $3 = $38.58333
    //~ //Test operator /
    //~ cout << "m13 / n = $" << m13 / n << endl;
    return 0;

}

Apparently, I get the answer : m7 + m8 = $4.94066e-324. Whereas the answer should be $38.05.

I've been stuck here for quite a while now. If anyone could patiently explain where I messed up that would be great. Thank you for your help.

Upvotes: 1

Views: 4157

Answers (2)

PaulMcKenzie
PaulMcKenzie

Reputation: 35440

The issue is your operator << outputs the amount member. However it is uninitialized:

// Define the output operator
ostream& operator<<(ostream& outputStream, const Money& money)
{
    outputStream << money.amount;  // not initialized
    return outputStream;
}

If you look at your cout in main, you have this:

(m7 + m8)

This returns a temporary Money object, which will be copy constructed (not default constructed). The copy constructor is the compiler-generated one (which is ok, but see my comments below). Since m7 nor m8 has set amount, you copy a garbage value to the temporary Money object, thus the garbage value.


Also, your code invokes undefined behavior due to the double variable amount not being initialized, and the compiler generated copy constructor copying a garbage double value to another double. You never want to try and manipulate an uninitialized floating point variable, unless the manipulation involves initializing the variable.

The bottom line is that when you construct your object, you should strive to initialize all members to a defined state. Whether it's making pointers NULL, whether it's making doubles equal to 0, etc. your member variables should be set to a valid state.

Upvotes: 0

Paul Rooney
Paul Rooney

Reputation: 21609

The constructor overload you are using in this case does not set 'amount'.

Money::Money(int newDollars, int newCents)
{
    dollars = newDollars;
    cents = newCents;
}

neither does your operator+

Money operator+(const Money& firstAmount, const Money& secondAmount)
{
    //assume cents < 100
    int carry = 0;
    int finalCents = firstAmount.cents + secondAmount.cents;

    if (finalCents >= 100){
        carry += 1;
        finalCents -= 100;
    }
    int finalDollars = firstAmount.dollars + secondAmount.dollars + carry;

    return Money(finalDollars, finalCents);
}

Your operator<< displays amount and it is uninitialised.

You should set amount in all your constructor overloads. This might be best achieved by having them all call a private function that does the init in one place.

Even better get rid of this duplicity of storing the value in 2 representations (dollars/cents & amount). Store it only as one or the other and it will become far simpler to maintain.

Note also that if you call the no args constructor your dollars and cents member variables are likewise uninitialised.

Upvotes: 1

Related Questions