Endless While Loop using classes

[project science]

I've created a code which takes in 2 fractions, and performs various operations on it, e.g. add, subtract, multiply, divide. It then will simplify the fraction for its final form.

I've been able to get it to work for add and subtract, but it goes in an endless loop for multiply when called in the member function get_reduced_fraction(). It appears something if going wrong in the if ((numerator_total_set % a == 0) && (common_denominator % a == 0)) (I'm aware there are some other areas I can clean up, but trying to get the basics taken care of first.)

main.cpp

/***** LIBRARIES AND HEADERS *****/
#include <iostream>

//using namespace std;
using std::cout;
using std::cin;
using std::endl;

#include "rational.h"  // header file which contains class definition for Shapes class

int main()
{	
	/* The overload constructor provides the class Rational with user-supplied parameters (where used, otherwise default constructor parameters are used)
	   When using default constructor arguments, at least 1 parameter has to be set
	 */

	//objects t* of class Rational
	Rational t0; // printing default dimensions of fractions, and getting reduced fractions.  member functions which have 0 arguments use object "t0"
	Rational t1; // pass values to member function set_add
	Rational t2;
	Rational t3;

	t0.get_dimensions();
	t3.combine_fractions(7, 2, 4, 6);  // integers are input, fractions are set
	//t2.get_reduced_fraction();

	//t1.set_add(7,2,4,6);
	t1.set_add();
	t2.get_reduced_fraction();

	t1.set_subtract();
	t2.get_reduced_fraction();

	t1.set_multiply();
	t2.get_reduced_fraction();

	//t1.set_subtract(7, 2, 4, 6);
	//t2.get_reduced_fraction();
	//t3.combine_fractions(7, 2, 4, 6);


	
	cout << "\n\n Press any key to close window...\n\n";
	// cin.get(); // keep terminal open until you press a key

	return 0;
}

header file

#ifndef RATIONAL_H 
#define RATIONAL_H

/*
Create a class, Rational.

The class member functions are placed in 'public'
The class member variables are defined in private.
*/

class Rational
{
	public:
		//default constructor
		Rational(int = 1, int = 1, int = 1, int = 1); // Rational default constructor initializes class Shapes's private data members


		/***** SET functions *****/
		int set_add();
		int set_subtract();
		int set_multiply();

		//int set_add(int, int, int, int); // add 2 Rational numbers (add 2 fractions)
		//int set_subtract(int, int, int, int);
		//int set_divide(int, int, int, int);
		

	    
		/***** GET functions (printing results) *****/
		void combine_fractions(int, int, int, int);
		void get_dimensions();  // show dimensions which are input
		void get_reduced_fraction();  //(int, int);  // takes a fraction and puts it in reduced form
		//void_get_decimal(); // prints fraction in decimal form



	private:

		//fraction 1
		int numerator_1;
		int denominator_1;
				
		//fraction 2
		int numerator_2;
		int denominator_2;


};


#endif
#pragma once 

member function file

#include <iostream>


//using namespace std;
using std::cout;
using std::endl;

#include <iomanip>
using std::setw;

#include "rational.h"  // include definition of class Rational from rational.h


/***** GLOBAL VARIABLES *****/
int numerator_total;

int num1, num2;
int common_denominator;

int numerator_total_set;

Rational::Rational(int N1, int N2, int D1, int D2)
{
	// fraction 1
	numerator_1 = N1;
	denominator_2 = D2;

	//fraction 2
	numerator_2 = N2;
	denominator_2 = D2;
}  // end of Rational constructor 



/***** Default Dimensions *****/
void Rational::get_dimensions()  // print all default dimensions (later change into using setwidth, precision, etc. for string
{
	// fraction 1
	cout << "\n***** DEFAULT DIMENSIONS *****" << endl;
	cout << "  numerator_1 = " << numerator_1 << endl;    // print numerator
	cout << "  denominator_2 = " << denominator_2 << endl;     // print denominator
	
	// fraction 2
	cout << "\n  numerator_2 = " << numerator_2 << endl;    // print numerator
	cout << "  denominator_2 = " << denominator_2 << endl;     // print denominator
	cout << "" << endl;  //skip a space

}
/***** COMBINING FRACTIONS *****/
void Rational::combine_fractions(int N1, int N2, int D1, int D2)
{	
	//pass resulting numberator and denominator to the function to reduce the fraction
		 // the below is set globally, or have another function called to use

	//fraction 1
	numerator_1 = N1;
	denominator_1 = D1;

	//fraction 2
	numerator_2 = N2;
	denominator_2 = D2;

	// get common denominator, make global so other member functions can use
	common_denominator = denominator_1 * denominator_2;

	// cross multiple numerators by opposite denominators, make global so other member functions can use
	    // used for :  Add, Subtract, Divide (not multiply)
	num1 = numerator_1 * denominator_2;
	num2 = numerator_2 * denominator_1;

	
	// print fractions to use  (these fractions have common denominators)
	cout << "\n***** COMBINING FRACTIONS *****" << endl;
	cout << "\nfraction 1 = " << num1 << " / " << common_denominator << endl;
	cout << "fraction 2 = " << num2 << " / " << common_denominator << endl;
	

	cout << "\n " << endl;

}





/******************
		ADD
******************/

int Rational::set_add()
{
			
	// add and prints combined fractions
	cout << "\n***** ADD FRACTIONS *****" << endl;		
	numerator_total_set = num1 + num2;
	cout << "\nAdded fractions = " << numerator_total_set << " / " << common_denominator << endl;
	cout << endl;
	
	return numerator_total_set;

}

/******************
	SUBTRACT
******************/


  //int Rational::set_subtract(int N1, int N2, int D1, int D2)
  int Rational::set_subtract()
	{
		cout << "\n***** SUBTRACT FRACTIONS *****" << endl;
		numerator_total_set = num1 - num2;
		cout << "\nAdded fractions = " << numerator_total_set << " / " << common_denominator << endl;
		cout << endl;

		return numerator_total_set;

	}
  
 
/******************
	 MULTIPLY
******************/
  
  int Rational::set_multiply()
  {
	  cout << "\n***** MULTIPLY FRACTIONS *****" << endl;
	    
	  numerator_total_set = num1 * num2;
	  common_denominator = common_denominator * common_denominator; // only used with multiplying fractions
	  cout << "\nMultiply fractions = " << numerator_total_set << " / " << common_denominator << endl;
	  cout << endl;

	  return numerator_total_set;

  }



******************
REDUCING FRACTIONS
******************/
       
	// this needs to take in final value of numerator and denominator, and reduce it.  It's to be done AFTER adding ,subtracting ,etc.
      
  void Rational::get_reduced_fraction()//(int numerator, int denominator)  // Note:  Have to figure out how to pass the vars in here
  {    
	  int a = 2; //test number
	  int TF = common_denominator; // TF is "terminating fraction", default setting
	  int numerator_reduced;
	  int denominator_reduced;

	  // begin steps to reduce fraction to simplest form

	  cout << "***** REDUCING FRACTIONS *****" << endl;

		  // find which number is smaller, numerator or denominator (default is denominator)
	  if (numerator_total_set < common_denominator)  // default setting 0, indicating denominator is smaller, but checking to see if 'while' stop condition should be switched
	  {
		  TF = numerator_total_set;		  
	  }

	  // use test numbers to see if, up to SMALLEST of the numerator or denominator, there is a common divisor
	  while (a < TF)
	  {
		 // cout << "terminationg fraction = " << TF << endl;
		 
		  /* 1.  test both numbers divisible by same number, terminating up to smallest number
		  *   2.  there will be no reminder in either case of even or odd if mod is 0
		  */

		  if ((numerator_total_set % a == 0) && (common_denominator % a == 0))
		  {
			  cout << "a = " << a << endl;
			  // write test result to temp int for each num and denom
			  numerator_reduced = numerator_total_set / a;
			  denominator_reduced = common_denominator / a;
			
			  // reset a
			  a = 2;
		  }

		  a++;  // increase 'a' to next test var		  
	  }

	  // print reduced fraction

	  cout << "\nreduced fraction = " << endl;
	  cout << setw(21) << numerator_reduced << endl;
	  cout << setw(21) << "__" << endl;
	  cout << endl;  // skip a space
	  cout << setw(21) << denominator_reduced << endl;
	

	  // also do get string length, and do math on it
  }

[jlb]

One of the first things I recommend is that you clean up the program and that you start by eliminating the global variables. IMO their use makes the logic too hard to follow.

[project science]

I didn't start out using global vars, but I saw it as the only way to pass these onto the member functions to use them. In the Combining Fractions function, I'm making 2 numerators and 1 common denominator, all found by cross multiplying. These are then set to global vars.

Member functions, e.g ADD, take the global vars in and perform math with the fractions. In this case, the fractions are added.

Then, that resultant fraction (using ADD as the example), is passed to the function 'Reducing Fractions', where the point is to simplify it. That's where the trouble begins in the While and If statement.

[helios]

I think you didn't quite understand how to design the class, which is why you ended up "needing" global variables.

Take a look at this. See if you can implement this class:

class Rational{
    int numerator;
    int denominator;

    //Helper function. Optional, but you'll probably need it.
    void reduce();
public:
    Rational(int n = 0, int d = 1);
    Rational(const Rational &) = default;            //Ignore
    Rational &operator=(const Rational &) = default; //Ignore
    //Returns the sum of *this and other.
    Rational add(const Rational &other) const;
    //Returns the subtraction of other from *this. I.e. *this - other
    Rational sub(const Rational &other) const;
    //Returns the product of *this and other.
    Rational mul(const Rational &other) const;
    //Optional. Returns the additive inverse of *this.
    Rational neg() const;
    //Optional. Returns the multiplicative inverse of *this (may fail).
    Rational rec() const;
};

[project science]

I don't quite understand what you're doing here, at least not yet. But it did spark some ideas.

What if I moved the global vars from the member function file, into the class header, and made them public class vars, like this:

#ifndef RATIONAL_H 
#define RATIONAL_H

/*
Create a class, Rational.

The class member functions are placed in 'public'
The class member variables are defined in private.
*/

class Rational
{
public:
	//default constructor
	Rational(int = 1, int = 1, int = 1, int = 1); // Rational default constructor initializes class Shapes's private data members

	// public variables
	int numerator_total;
	int num1, num2;   // each are set via cross multiplying the opposing fraction's denominator
	int common_denominator;  // set when multiplying each denominator
	int numerator_total_set; // set when adding num 1 + num 2

	/***** SET functions *****/
	int set_add();
	int set_subtract();
	int set_multiply();

	int set_add(int, int, int, int); // add 2 Rational numbers (add 2 fractions)

	//int set_subtract(int, int, int, int);
	//int set_divide(int, int, int, int);

	/***** GET functions (printing results) *****/
	void combine_fractions(int, int, int, int);
	void get_dimensions();  // show dimensions which are input
	void get_reduced_fraction();  //(int, int);  // takes a fraction and puts it in reduced form
	//void_get_decimal(); // prints fraction in decimal form

private:

	//fraction 1
	int numerator_1;
	int denominator_1;

	//fraction 2
	int numerator_2;
	int denominator_2;
};

#endif
#pragma once  

Then I was thinking, somehow, I'd be able to access and write to these public vars, using an object to publicly access them in main.cpp. Something like:

t4.numerator_total = combine_fractions(7, 2, 4, 6);
t4.numerator_total_set = combine_fractions(7, 2, 4, 6);
t4.common_denominator = combine_fractions(7, 2, 4, 6);
t4.num1 = combine_fractions(7, 2, 4, 6);
t4.num2 = combine_fractions(7, 2, 4, 6);

All the public class vars are now written to.

Then, also in main.cpp, I would then pass these vars into the function calls. Using ADD as an example,

int set_add(numerator_total_set, num1, num2, common_denominator);

This would pass these vars to the rewritten member function:

int set_add(numerator_total_set, num1, num2, common_denominator);
{
			
	// add and prints combined fractions
	cout << "\n***** ADD FRACTIONS *****" << endl;		
	numerator_total_set = num1 + num2;
	cout << "\nAdded fractions = " << numerator_total_set << " / " << common_denominator << endl;
	cout << endl;
	
	return numerator_total_set;
}

This would then return the mixed fraction with the public vars numerator_total_set and common_denominator being written to.

Finally, I would rewrite the member function for reducing fractions, call it from main.cpp, pass these 2 public class vars in where they're received in the member function, as:

int Rational::get_reduced_fraction(int numerator_total_set, int common-denominator)

Does this make sense, or am I just on a wild goose chase?

[helios]

Your class is poorly designed from the get-go. Think about what your class is representing. It's supposed to be a rational number, right? Why would you need two fractions to represent a single rational number? Every other problem in your code follows from this fundamental misunderstanding, from the nonsensical functions to the apparent need for global state.

[Manga]

line 26 of member function file, looks like a typo to me.

[project science]

I tried cleaning up my code, and taking one more pass at it before I scrap it and start over. I removed the global vars, and made a reduced version of it to get things right.

My sticking point seems to be passing the class’ private vars into the member functions. I cannot seem to be able to do that, and use it in the source code Rational::set_add() block.


main.cpp
I tried cleaning up my code, and taking one more pass at it before I scrap it and start over. I removed the global vars, and made a reduced version of it to get things right.

My sticking point seems to be passing the class’ private vars into the member functions. I cannot seem to be able to do that, and use it in the source code Rational::set_add() block.

main.cpp

/***** LIBRARIES AND HEADERS *****/
#include <iostream>

#include <iomanip>
using std::setw;

//using namespace std;
using std::cout;
using std::cin;
using std::endl;

#include "rational.h"  // header file which contains class definition for Rational class

int main()
{
    //objects t* of class Rational
    Rational t1;
    Rational t2;
                
    t1.set_combine_fractions(3, 6, 4, 8); // passes 4 integers to member function

    //t3.set_add(numerator_1, numerator_2, denominator_1);
    t2.set_add();

    cout << "\n\n Press any key to close window...\n\n";
    // cin.get(); // keep terminal open until you press a key

    return 0;
}

Header file

#ifndef RATIONAL_H
#define RATIONAL_H

class Rational
{
public:
    //default constructor
        Rational(int = 1, int = 1, int = 1, int = 1); // Rational default constructor initializes class Rational's private data members
                
    /***** SET functions *****/
        
        void set_combine_fractions(int, int, int, int);
        void set_add(int numerator_1, int numerator_2);
        

    /***** GET functions (printing results) *****/
        
private:

    //fraction 1
    int numerator_1;
    int denominator_1;

    //fraction 2
    int numerator_2;
    int denominator_2;

    // not written to yet
    int num_1;
    int num_2;
    int common_numerator;   
    int common_denominator;
};

#endif
#pragma once 

Source file

#include <iostream>

#include <iomanip>
using std::setw;

//using namespace std;
using std::cout;
using std::cin;
using std::endl;

#include "rational.h"  // header file which contains class definition for Rational's class


Rational::Rational(int N1, int N2, int D1, int D2)
{
    // fraction 1
    numerator_1 = N1;
    denominator_2 = D2;

    //fraction 2
    numerator_2 = N2;
    denominator_2 = D2;


    
    
    // fraction 1
    cout << "\n***** DEFAULT DIMENSIONS *****" << endl;
    cout << "  numerator_1 = " << numerator_1 << endl;    // print numerator
    cout << "  denominator_2 = " << denominator_2 << endl;     // print denominator

    // fraction 2
    cout << "\n  numerator_2 = " << numerator_2 << endl;    // print numerator
    cout << "  denominator_2 = " << denominator_2 << endl;     // print denominator
    cout << "" << endl;  //skip a space
        

}  // end of Rational constructor



void Rational::set_combine_fractions(int N1, int N2, int D1, int D2)
{
  // write NEW values to class private data members, which is provided from main.cpp
    //fraction 1
    numerator_1 = N1;
    denominator_1 = D1;

    //fraction 2
    numerator_2 = N2;
    denominator_2 = D2;

    //get common denominator
    common_denominator = denominator_1 * denominator_2;  

    //cross multiple numerators by opposite denominators
        // used for :  Add, Subtract, Divide (not multiply)
    num_1 = numerator_1 * denominator_2;
    num_2 = numerator_2 * denominator_1;


    // print fractions to use  (these fractions have common denominators)
    cout << "\n***** COMBINING FRACTIONS *****" << endl;
    cout << "\nfraction 1 = " << num_1 << " / " << denominator_1 << endl;
    cout << "fraction 2 = " << num_2 << " / " << denominator_1 << endl;

    cout << "\n " << endl;
    
}

void Rational::set_add(int numerator_1, int numerator_2)
{
    // add and prints combined fractions
    cout << "\n***** ADD FRACTIONS *****" << endl;
    common_numerator = numerator_1 + numerator_2;
    cout << "\nAdded fractions = " << common_numerator << " / " << common_denominator << endl;
    cout << endl;

    //return numerator_1;
}

[jonnin]

ideally, if you know it, you would overload the addition (+) operator. Are you allowed to do this?

If not, you would not pass private members, just pass the object:

rational add (rational &other)
{
rational result;
...
return result;
}

it is ok inside your add routine to access other.numerator and other private variables.
typically something like 'add' would return a valued result, not modify what came in (that is sort of a += behavior, you can do it, but the user may be unexpecting it).

[againtry]

You need to consider reviewing your code as the solution can be greatly simplified if you concentrate on a single rational number instead of two as your class. Don't concern yourself at this stage about const, &, *, and/or operator overloads, especially if it doesn't mean much to you insofar as where you are in your studies.

subtract() and multiply() functions should be pretty easy to add as they follow the same pattern as add(). divide() is pretty easy too.

#include <iostream>

using namespace std;


class Rational
{
public:
    Rational(int = 1, int = 1);
    
    Rational add(Rational);
    void print();

private:
    int numerator;
    int denominator;
};



Rational::Rational(int N, int D)
{
    numerator = N;
    denominator = D;
}

Rational Rational::add(Rational r)
{
    
    Rational result;
    result.numerator
    = this->numerator * r.denominator + (r.numerator * this->denominator);
    
    result.denominator = this->denominator * r.denominator;
    
    return result;
}

void Rational::print()
{
    cout << numerator << '/' << denominator << '\n';
}




int main()
{
    Rational t1(3,4);
    Rational t2(5,8);
    
    Rational t3 = t1.add(t2);
    
    t3.print();
    
    return 0;
}