Thread: New to classes with dynamic memory, looking for some help

  1. #1
    Registered User
    Join Date
    Dec 2010
    Posts
    19

    New to classes with dynamic memory, looking for some help

    So I'm pretty new to classes in general, and I'm just now making my first attempt at a class with dynamic memory allocation. I made a Rational class that does arithmetic on fractions, with private variables of numerator and denominator. It worked fine without using dynamic memory, but I'm now trying to use dynamic memory and I'm having some problems. I've written the class definition and implementation files, and now I'm testing it with a separate file, but the program crashes as soon as I use one of the arithmetic functions. I can't see any problems in them, so I'm thinking that the problem has to do with my constructors. My book doesn't tell me much about copy constructors, so I may have done something wrong in there. Anyway, here's my code, and if anyone can point out what's going wrong, or point me in the right direction, that'd be great.

    Class definition file:

    Code:
    #ifndef RATIONAL_H
    #define RATIONAL_H
    
    class Rational
    {
    	
    public:
    	Rational(int = 1, int = 1); //Default constructor, sets values to 1/1
    	Rational(const Rational &);
    	~Rational();
    
    	void set_fraction(int, int);
    	void simplify(); //Simplifies
    		
    	Rational operator +(const Rational &) const; //Add
    	Rational operator -(const Rational &) const; //Subtract
    	Rational operator *(const Rational &) const; //Multiply
    	Rational operator /(const Rational &) const; //Divide
    	Rational & operator =(const Rational &);
    	Rational & operator ++();
    	Rational operator ++(int);
    	Rational & operator +=(const Rational &);
    	Rational & operator -=(const Rational &);
    	Rational & operator *=(const Rational &);
    	Rational & operator /=(const Rational &);
    	bool operator ==(const Rational &) const;
    	bool operator !=(const Rational &) const;
    
    	int get_numerator() const
    	{
    		return *numerator;
    	}
    	int get_denominator() const
    	{
    		return denominator;
    	}
    	
    private:
    	int *numerator;
    	int denominator;
    	void increment();
    };
    
    #endif
    Class implementation file:

    Code:
    #include <iostream>
    #include <iomanip>
    #include <cmath>
    #include "Rational.h"
    using namespace std;
    
    Rational::Rational(int num, int denom)
    	: numerator(new int(0))
    {
    	set_fraction(num, denom);
    }
    
    Rational::Rational(const Rational &right)
    {
    	set_fraction(*(right.numerator), right.denominator);
    }
    
    Rational::~Rational()
    {
    	delete numerator;
    	numerator = 0;
    }
    
    void Rational::set_fraction(int num, int denom)
    {
    	if (!denom)
    	{
    		cout << denom << " cannot be a denominator.  Changing to 1" << endl;
    		denominator = 1;
    	}
    	else
    		denominator = denom;
    
    	if (numerator)  //If the numerator does not point to NULL, it will be deleted
    		delete numerator;
    
    	numerator = new int(num);
    
    	if (*numerator) //If numerator is anything but 0
    		simplify();
    }
    
    void Rational::simplify() 
    {
    	if (*numerator < 0 && denominator < 0) //Check for both being negative
    		{
    			*numerator = abs(*numerator);
    			denominator = abs(denominator);
    		}
    
    	int k;
    	for (k = abs(*numerator); 0 < k; k--)
    	{
    		if (*numerator % k == 0 && denominator % k == 0) 
    			break; //Stops at the greatest common denominator
    	}
    	*numerator = (*numerator) / k; //Divides both numbers by the GCD
    	denominator = denominator / k;
    }
    
    Rational Rational::operator +(const Rational &right) const
    {
    	Rational temp; //Temporary object to hold the result
    
    	temp.set_fraction(((*numerator * right.denominator) + (denominator * *(right.numerator))), (denominator * right.denominator));
    	
    	return temp;
    }
    
    Rational Rational::operator -(const Rational &right) const
    {
    	Rational temp;
    
    	temp.set_fraction(((*numerator * right.denominator) - (denominator * *(right.numerator))), (denominator * right.denominator));
    	
    	return temp;
    }
    
    Rational Rational::operator *(const Rational &right) const
    {
    	Rational temp;
    
    	temp.set_fraction((*numerator * *(right.numerator)), (denominator * right.denominator));
    
    	return temp;
    }
    
    Rational Rational::operator /(const Rational &right) const
    {
    	Rational temp;
    
    	temp.set_fraction((*numerator * right.denominator), (denominator * *(right.numerator)));
    
    	return temp;
    }
    
    Rational & Rational::operator =(const Rational &right)
    {
    	if(this == &right)
    		return *this;
    	
    	set_fraction(*(right.numerator), right.denominator);
    	return *this;
    }
    
    Rational & Rational::operator ++()
    {
    	increment();
    	return *this;
    }
    
    Rational Rational::operator ++(int)
    {
    	Rational temp(*this);
    	increment();
    	return temp;
    }
    
    Rational & Rational::operator +=(const Rational &right)
    {
    	*this = *this + right;
    	return *this;
    }
    
    Rational & Rational::operator -=(const Rational &right)
    {
    	*this = *this - right;
    	return *this;
    }
    
    Rational & Rational::operator *=(const Rational &right)
    {
    	*this = *this * right;
    	return *this;
    }
    
    Rational & Rational::operator /=(const Rational &right)
    {
    	*this = *this / right;
    	return *this;
    }
    
    bool Rational::operator ==(const Rational &right) const
    {
    	if(*numerator == *(right.numerator) && (denominator == right.denominator))
    		return true;
    	else
    		return false;
    }
    
    bool Rational::operator !=(const Rational &right) const
    {
    	if(*numerator != *(right.numerator) || (denominator != right.denominator))
    		return true;
    	else
    		return false;
    }
    
    void Rational::increment()
    {
    	int temp = *numerator;
    	delete numerator;
    	temp += denominator;
    	numerator = new int(temp);
    }
    Test program, causes it to crash:

    Code:
    #include <iostream>
    #include <iomanip>
    #include "Rational.h"
    using namespace std;
    
    int main()
    {
    	Rational r1(5, 3), r2(8, 3);
    	
            r1 + r2;
    
    	cout << "Success" << endl;
    	return 0;
    }
    So yeah, if you can see what's wrong with it, please let me know. Thanks.

  2. #2
    Registered User
    Join Date
    Jun 2005
    Posts
    6,815
    Your copy constructor needs to dynamically allocate numerator.
    Right 98% of the time, and don't care about the other 3%.

    If I seem grumpy or unhelpful in reply to you, or tell you you need to demonstrate more effort before you can expect help, it is likely you deserve it. Suck it up, Buttercup, and read this, this, and this before posting again.

  3. #3
    Registered User
    Join Date
    Dec 2010
    Posts
    19
    I thought that it was doing that with the set_fraction function it called. Is it not?

  4. #4
    Registered User
    Join Date
    Jun 2005
    Posts
    6,815
    Not properly, no.

    The copy constructor does not initialise numerator at all - the value is not guaranteed to be zero or NULL, for example. set_fraction() then accesses the value of numerator (in testing if it is non-null, and deciding whether to delete, and then reallocate).

    set_fraction() assumes numerator is either NULL, or has been previously allocated using operator new. The copy constructor does not satisfy that assumption.

    The simple act of accessing the value of an uninitialised pointer gives undefined behaviour. If the value of the pointer happens to test as non-NULL (which IS within the range of possibilities) then deleting it also gives undefined behaviour.

    "Undefined" in the C++ standard has a very specific meaning: anything is allowed to happen. Such as a crash, a value you expect to be zero not being zero, etc.
    Last edited by grumpy; 03-08-2011 at 02:41 AM.
    Right 98% of the time, and don't care about the other 3%.

    If I seem grumpy or unhelpful in reply to you, or tell you you need to demonstrate more effort before you can expect help, it is likely you deserve it. Suck it up, Buttercup, and read this, this, and this before posting again.

  5. #5
    Registered User
    Join Date
    Dec 2010
    Posts
    19
    Well, I'll be darned. Thank you for the nice explanation, it's working now.

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Dynamic memory and realloc(), freeing memory
    By C_Sparky in forum C Programming
    Replies: 6
    Last Post: 10-06-2010, 07:55 PM
  2. Why use Dynamic Memory
    By appleGuy in forum C++ Programming
    Replies: 11
    Last Post: 08-28-2006, 02:46 PM
  3. Dynamic memory allocation...
    By dicorr in forum C Programming
    Replies: 1
    Last Post: 06-24-2006, 03:59 AM
  4. Inheritance and Dynamic Memory Program Problem
    By goron350 in forum C++ Programming
    Replies: 1
    Last Post: 07-02-2005, 02:38 PM
  5. Dynamic memory allocation
    By amdeffen in forum C++ Programming
    Replies: 21
    Last Post: 04-29-2004, 08:09 PM