Painfully simple problem with validation

[SSully]

So simple problem. I need to have a division by zero validation in my constructor in this class. It is painfully simple, but I am just starting to learn classes and I just cannot figure out what I have to do.

Would it be easier to just put the validation in my RationalNumber constructor, or a constructor for the validation separate??

Any help would be greatly appreciated.

#ifndef RATIONALNUMBER_H
#define RATIONALNUMBER_H

// The FeetInches class holds distances or measurements 
// expressed in feet and inches.

class RationalNumber
{
private:
   int numerator;       // To hold a number of feet
   int denominator;     // To hold a number of inches
   void reduce();		// Defined in FeetInches.cpp
public:
   // Display function
	void display();

	// Constructor
   RationalNumber(int n = 0, int d = 1)
   {
	   if (d == 0)
	   {
		   denominator = 1;
	   }
	   numerator = n;
	   denominator = d;
	   reduce();
   }
  

   // Mutator functions
   void setNumerator(int n)
      { numerator = n; }

   void setDenominator(int d)
      { denominator = d;
        reduce(); }

   // Accessor functions
   int getNumerator() const
      { return numerator; }

   int getDenominator() const
      { return denominator;}
  
   
};

#endif

[Kanoisa]

Hi,

I think check if its 0 and then changing it to a non 0 value is the way to go, but look again at your code

if (d == 0) //if d ==0 then make it non 0
{
    denominator = 1;
}
numerator = n;
denominator = d; //assign denominator to d again ,... which may be 0!!

remove the line denominator = d and i think thats as good as anything really.

[SSully]

Thanks for the response.

I tried removing the suggested line and it still does not work. When I run the program and enter my numerator and denominator (with a zero) it does not replace the zero with a 1 and thus resulting in a crash due to division by zero.

[Kanoisa]

Are you using this function?

void setDenominator(int d)
{ 
    denominator = d;
    reduce(); 
}

because it does not include the value checking and could result in this happening, but otherwise if you get divide by 0 from the constructor i posted i would need to see your reduce function and would suspect the isue is there not in the constructor.

[SSully]

Yes, I am using that function.

void RationalNumber::reduce()
{
	int largest, gcd = 1; // greatest common divisor;
	largest = ( numerator > denominator ) ? numerator: denominator;
	for ( int loop = 2; loop <= largest; loop++ )
	if ( numerator % loop == 0 && denominator % loop == 0 )
	gcd = loop;
	numerator /= gcd;
	denominator /= gcd;
}

This is my reduce function.

[Kanoisa]

ok so a few things then,

1 the function void setDenominator(int d) needs the same protection as the construtor, also the compiler isnt clever, so make life easier a line of code like

if ( numerator % loop == 0 && denominator % loop == 0 )

there is no guarintee the compiler is doing that you think here, so based on what i think you are trying to do bracket terms to show the logical steps. This also makes it clear to others what your code is doing, and also to you when you come back to it later. Also i am of the club that believes all if's should have {} regardless if its one line or not. so i would suggest the following

if ( ( (numerator % loop) == 0 )&&( (denominator % loop) == 0 ) )
{
    /* do something */
}

also please note your for loop has no {} either so it makes your code very hard to follow, and my bet is that your code does not do what you expect atall.