please fix this so it works

J

John Harrison

Luis said:
can some one fix this so it actually reduces a fraction, and actually works?
I get a windows error when i try to run it.

http://www.rafb.net/paste/results/wS672281.html

Your code is confused, it can't be easily fixed. Look at the code for GCF

int Fraction :: GCF()
{
int remainder = 1;
int gcf;
while(remainder!=0)
{
remainder = denominator%numerator;
denominator = numerator;
numerator = remainder;
}
gcf = denominator;
return gcf;
}

Why is GCF a member of Fraction? What does calculating a GCF have to do with
fractions? More to the point why does GCF change the numerator and
denominator? Its the job of Reduce to change the numerator and denominator
by dividing by the GCF. Its not the job of GCF to do that. As I said you
are confused.

First job, write this function

int GCF(int x, int y)
{
// code to calculate GCF of x and y
return gcf;
}

Not this function is not a member of Fraction, its doesn't have anything to
do with Fractions, all it does it calculate a GCF. This is a good rule for
programming; one function, one job.

Now look at Reduce

Fraction Fraction :: Reduce ()
{
Fraction reducedFraction;
reducedFraction.set(numerator/GCF(),denominator/GCF());
return reducedFraction;
}

You've written Reduce to return a new Fraction, not reduce the existing one.
That's not wrong but its a bit strange. Its seems much more logical to
reduce the existing fraction When I look at DividedBy I see more evidence of
this confusion.

Fraction Fraction :: DividedBy (Fraction otherFraction)
{
Fraction reducedFraction;
Fraction quotient(numerator * otherFraction.denominator,
denominator * otherFraction.numerator);
Reduce();
return quotient;
}

Which fraction to do think is being reduced by the call to Reduce here?
reducedFraction? quotient? this? The answer is none of them! You wrote
Reduce to create a new Fraction which it returns, but when you call Reduce
here, you throw the return value away! You call to Reduce does nothing at
all (apart from waste some time).

Lets rewrite reduce so that it reduces the existing fraction, not returns a
new one, also let is use the GCF function I said you should write above.

void Fraction :: Reduce ()
{
int gcf = GCF(numerator, denominator);
numerator /= gcf;
denominator /= gcf;
}

See how it deivdes the existing Fractions numerator and denominator, its
doesn't try to create a new Fraction. The return type is void.

Now let rewrite DividedBy to use the new Reduce

Fraction Fraction :: DividedBy (Fraction otherFraction)
{
Fraction quotient(numerator * otherFraction.denominator,
denominator * otherFraction.numerator);
quotient.Reduce();
return quotient;
}

See how I call quotient.Reduce() to reduce quotient, simple!

Carry on in this vein, and you'll be getting closer. Its very important to
think clearly about you program design, you cannot throw together a few
functions that look roughly right and expect to get anything to work.

john
 
A

Allan Bruce

Luis said:
can some one fix this so it actually reduces a fraction, and actually works?
I get a windows error when i try to run it.

http://www.rafb.net/paste/results/wS672281.html

In your reduce function you have:

reducedFraction.set(numerator/GCF(),denominator/GCF());

you call numerator/GCF() first then denominator/GCF()
The two GCF() calls will return different values, instead execute GCF() once
and store it and then divide the two, i.e

gcf = GCF();
reducedFraction.set(numerator/gcf, denominator/gcf);

HTH
Allan
 
A

Allan Bruce

Mark said:
Please don't post homework so that others can do it for you

Hmm top posting - I was told off for that when I started out here.
I think a bit more constructive effort is required - at least the OP has
attempted something, other than just asking someone to do it from scratch.
Allan
 
J

John Harrison

Allan Bruce said:
In your reduce function you have:

reducedFraction.set(numerator/GCF(),denominator/GCF());

you call numerator/GCF() first then denominator/GCF()
The two GCF() calls will return different values, instead execute GCF() once
and store it and then divide the two, i.e

gcf = GCF();
reducedFraction.set(numerator/gcf, denominator/gcf);

HTH
Allan

But his GCF function also changes the Fractions state. Its very confused
coding which doesn't have a quick fix.

john
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

No members online now.

Forum statistics

Threads
474,139
Messages
2,570,805
Members
47,351
Latest member
LolaD32479

Latest Threads

Top