I am trying to make this project and im very new to this but i really want to learn this can anyone check this code for me and tell me whats wrong with it. I have to get it to add 2 fractions display result then display result in decimal form Thanks.

//Rational class interface


#ifndef RATIONAL_H
#define RATIONAL_H

class Rational
{
public:
	Rational(int numerator = 0,int denominator = 1);//default constructor
	int setTotal;
	Rational add(const Rational&);//add prototype
	Rational subtract(const Rational&);//subtract prototype
	Rational multiply(const Rational&);//multiply prototype
	Rational divide(const Rational&);//divide prototype
	void printRational();//function prototype
	void printRationalAsDouble();//function prototype
    void reduction();//utility prototype
	int setNumerator;
	int setDenominator;

private:
	int numerator;//data member
	int denominator;//data member
	
   
};
#endif
// Lab5.cpp : Defines the entry point for the console application.
//

#include "stdafx.h"
#include "Rational.h"
#include <iostream>
#include<cmath>
using namespace std;
int numerator;
int denominator;
int t;
int total;
int setNumerator;
int setDenominator;

Rational::Rational(int n,int d)
{
	numerator = d < 0 ? -n : n;
	denominator = d < 0 ? -d : d;
	reduction();
}

Rational Rational::add(const Rational& a)
{
    Rational t;
	t.numerator = a.numerator * denominator + a.denominator * numerator;
	t.denominator = a.denominator * denominator;
	t.reduction();
	return t;
}

Rational Rational::subtract(const Rational& s)
{
	Rational t;
	t.numerator = s.numerator * denominator - s.denominator * numerator;
	t.denominator = s.denominator * denominator;
	t.reduction();
	return t;
}

Rational Rational::multiply(const Rational& m)
{
	Rational t;
	t.numerator = m.numerator * numerator;
	t.denominator = m.denominator * denominator;
	t.reduction();
	return t;
}

Rational Rational::divide(const Rational& v)
{
	Rational t;
	t.numerator = v.denominator * numerator;
	t.denominator = denominator * v.numerator;
	t.reduction();
	return t;
}
void Rational::printRational()
{
	if(denominator == 0)
		cout<<"DIVIDE BY ZERO ERROR"<<endl;
	else if(numerator == 0)
		cout<<0;
	else
		cout<<numerator<<"/"<<denominator<<endl;
}
void Rational::printRationalAsDouble()
{
	static_cast<double>(numerator / denominator);	
			
}
void Rational::reduction()
	{
		int largest = numerator > denominator ? numerator : denominator;
		int gcd = 0;
	for(int loop = largest; loop >= 2; loop--)
		if(numerator % loop == 0 && denominator % loop == 0)
			gcd = loop;
	    else if(gcd != 0)
		numerator /= gcd;
		denominator /= gcd;
	}

int _tmain(int argc, _TCHAR* argv[])
{	
	Rational obj1,obj2,obj3;
	{
		obj1 = numerator,denominator;
		obj2 = numerator,denominator;
		obj3 = t;
	}
	
		
	
	return 0;
}

Hi Mike - welcome to DaniWeb!

Can you detail the problem more for us? What is wrong? Are there are any compiler errors? Give an example input, the current output, and the expected output.

Looking at this code:

Rational obj1,obj2,obj3;
	{
		obj1 = numerator,denominator;
		obj2 = numerator,denominator;
		obj3 = t;
	}

The first line declares three instances of the Rational class. The braces are unnecessary.

As far as I know, this syntax is not valid:

obj1 = numerator,denominator;

what are you trying to do here?

Good luck,

Dave

The big problem other than how you are trying to initialize your variables would be the fact that it is dividing by zero. I commented the code where it was wrong.

void Rational::reduction()
{
	int largest = numerator > denominator ? numerator : denominator;
	int gcd = 0;
	for(int loop = largest; loop >= 2; loop--)
		if(numerator % loop == 0 && denominator % loop == 0)
			gcd = loop;
   	else if(gcd != 0)
	{
		numerator /= gcd;
		denominator /= gcd; //problem was here because this if statement was not in brackets (was always dividing by gcd at the end and if it was reduced then it was divided by zero)
	{
}

I don't see why gcd would ever be 0. Its minimum should be 1, because 1 can divide into
all natural numbers evenly. Also look at the for loop condition, gcd can never reach 0.

I used 6 for numerator and 1 for denominator and it was crashing because it was trying to divide by 0 when the if statement was not closed because 6 can never go into 1 because he has it stop at 2.

First off the most scary thing that I see is the global variables defined just below your line using namespace std;. Arrrhh.... don't do this, they are not needed.

Next reduction is missing a few brackets.

Yes gcd CAN be zero if say numerator and denominator are 1.

Now the algorithm used is a mess. It is easier to count up, than down.
You will not found a common factor that is bigger than the SMALLEST of the
numerator and denominator, e.g. 1/387642212 is not going to be reduced
regardless of the factors in the denominator.

So the algorithm you need:

Loop I: from 2 to smallest number
    while (I is common factor)
        divided denominator and numerator
    recalc smallest number

Note: It is very important to check a factor many times, e.g. consider 16/32,
2 is a factor but you can divide by 2, four times.

Improvements of this algorithm include : using a prime sieve, so you only have
to test primes, stepping up in 2s, looping to the sqrt(smallest) , but remembering
to test the smallest number as well.

>>Yes gcd CAN be zero if say numerator and denominator are 1.

gcd(1/1) = 1;

Why would anyone start counting from 0 and up, instead of 1 and up? Maybe its getting
too late. I'm going to sleep.

Sorry firstPerson, I think I miss-read what you were saying and then failed to explain my comment.
The code as presented can allow gcd [the variable] to be zero. [hence the test in the code, etc].

I fully accept that nobody should start with zero (or one) but most likely from 2 and go up.
And you are correct that gcd cannot be zero.

I also think that this reduction method should be deleted and re-written using a better algorithm.

Anyway, sorry to all those I confused. I think my original post is mostly sane after that statement... (hopefully) .

Edited 6 Years Ago by StuXYZ: n/a

ok thank you all for your input ill go over it and let you know if there are any problems

This question has already been answered. Start a new discussion instead.