Segmentation Fault when using overloaded operators and member pointers

Question

So I am fairly new to coding in C++ and in my current programming course we are learning about operator overloading and friend functions. We were told to make a class Money that has different types of constructors and overloaded operators. The program was much easier when we didn't have pointers for our private member variables, but now it's a little over my head.

I wanted to get some help on this before I continued on defining the rest of the overloaded operator functions. Basically what I'm trying to do is add two objects of the Money class together, but I keep on getting a segmentation fault when I run the program. I know this has to do with pointers and accessing memory that can't be accessed, but I'm not sure where I went wrong.

It's a short program so far, so the code should be easy to read. Any help would be appreciated!

class Money
{
public:
        Money(int d=0, int c=0);
        Money(const Money&);
//      ~Money();

        Money& operator=(const Money&);
        Money operator+(const Money&) const;
        Money operator-(const Money&) const;
        Money& operator*(double);
        Money& operator/(double);

        friend istream& operator>>(istream&, Money&);
        friend ostream& operator<<(ostream&, const Money&);
private:
        int* dollars;
        int* cents;
};

int main()
{
        Money m1(3, 43), m2(4, 64);

        Money m3 = m1 + m2;

        return 0;
}

Money::Money(int d, int c)
{
        *dollars = d;
        *cents = c;
}


Money Money::operator+(const Money& m1) const
{
        Money result;
        *result.dollars = *this->dollars + *m1.dollars;
        *result.cents = *this->cents + *m1.cents;
        return result;
}

Why do you define dollars and cents as pointers to integers? That's a bad idea. You try to use these integers (derefence the pointer) without actually initialising them. — NineBerry, Oct 17 '17 at 22:24
Are you required to use int* for dollars and cents? Use plain int instead if your assignment allows it. — Darryl, Oct 17 '17 at 22:26
I am required to use pointer to int for dollars and cents. I wish I could! Still learning the ins and outs of dynamic memory allocation. — Dane Wind, Oct 17 '17 at 22:29
@DaneWind _"I am required to use pointer to int for dollars and cents"_ That's complete nonsense, tell your professor please. — user0042, Oct 17 '17 at 22:37
@DaneWind Please re-read your question this is not a good place for pointers. — Martin York, Oct 17 '17 at 22:46
Stand with the others on this one. Using pointers here is an amazingly bad idea. Either this assignment is teaching a flawed lesson or you are misunderstanding it. I'd read the assignment spec again and consult the instructor. To do this with pointers you need to dynamically allocate with `new`, free the allocations with `delete` and observe the Rule of Three. [What is the Rule of Three?](https://stackoverflow.com/questions/4172722/what-is-the-rule-of-three) Read the link and find out. — user4581301, Oct 18 '17 at 00:14

frslm · Answer 1 · 2017-10-18T00:43:23.107

2

You haven't allocated memory for dollars and cents. You should do this in your constructor:

Money::Money(int d, int c)
{
    dollars = new int;
    cents = new int;

    *dollars = d;
    *cents = c;
}

And don't forget to release the memory in the destructor:

Money::~Money() {
    delete dollars;
    delete cents;
}

Otherwise, you've simply declared you have two pointers to ints (dollars and cents), but you haven't actually ensured that they point to valid spots in memory; they're uninitialized.

Hint: Try to keep this resource management in mind when implementing your copy constructor (Money(const Money&)) and operators (thanks goes to @LokiAstari for being thorough).

edited Oct 18 '17 at 00:43

answered Oct 17 '17 at 22:25

frslm

2,969
3
13
26

1

Thank you very much. I'm still learning a lot about pointers and i guess I completely skipped over this. – Dane Wind Oct 17 '17 at 22:29
1

@DaneWind Why do you need pointers for simple `int` variables at all? – user0042 Oct 17 '17 at 22:35
1

This advice makes the problem worse. – Martin York Oct 17 '17 at 22:44
@LokiAstari: Until OP confirms that they don't actually need `dollars` and `cents` to be pointers, this answer should do. – frslm Oct 17 '17 at 23:22
1

@frslm Disagree. Your solution violates the rule of three and thus causes many more issues than it solves. – Martin York Oct 17 '17 at 23:47
@LokiAstari: OP has yet to implement the copy constructor and copy assignment operator for `Money`; it would be unfair to write half their assignment for them. I had chosen to add the destructor code as OP might've thought not to implement anything for it (judging by the commented-out declaration). – frslm Oct 18 '17 at 00:21
@frslm: He does not need to implement them. They are constructed automatically by the compiler. This is exactly why your suggestion is so dangerous bad. – Martin York Oct 18 '17 at 17:09
@LokiAstari: Please read the question; he's required to: "We were told to make a class Money that has different types of constructors and overloaded operators." – frslm Oct 18 '17 at 17:12

Segmentation Fault when using overloaded operators and member pointers

1 Answers1