As a beginner of C++, i cannot handle this project smoothly, please correct my code!

Write a class of fraction. Your fraction should provide the methods read(), print()
and the friend operator +. The read() method will read from the keyboard the integral
part, the numerator and the denominator respectively. The print() method will print
the fraction in the form afb/cg.

Finding the greatest common divisor between the numerator and denominator. Use recursion to implement the gcd() function.

The class fraction will look like:
class fraction
{
public:
fraction(); //constructor for initialize
void read(); //read 3 int from keyboard for the fraction
void print(); //print the fraction on screen
friend fraction operator +(fraction, fraction); //fraction add
private:
int integral, numerator, denominator;
}

Write a program using this class to first read from the input the number of fraction to
be added. Then create a dynamic array of the fraction object, and perform addition.

================================

#include <cstdlib>
#include <iostream>
using namespace std;

int gcdr(int m, int n);
int gcd(int m, int n);

class fraction {
public:
    fraction(); //constructor for initialize
    void read(); //read 3 int from keyboard for the fraction
    void print(); //print the fraction on screen
    friend fraction operator +(fraction, fraction); //fraction add
private:
    int integral, numerator, denominator;
};

fraction::fraction() {
    integral, numerator, denominator = 0;
}

void fraction::read() {
    cin >> integral >> numerator >> denominator;
}

void fraction::print() {
    cout << integral << "{" << numerator << "/" << denominator << "}" << endl;
}

int gcd(int m, int n) {

    if (m == 0 && n == 0)
        return 0;
    if (m < 0)
        m = -m;
    if (n < 0)
        n = -n;

    return (gcdr(m, n));
}

int gcdr(int m, int n) {

    if (m == 0)
        return (n);
    if (n == 0)
        return (m);

    return (gcd(n, m % n));
}

fraction operator + (fraction, fraction) {
    fraction fraction1, fraction2, fraction_sum;
    int a, b, c, d, e, f, g, h, i;

    a = fraction1.integral;
    b = fraction1.numerator;
    c = fraction1.denominator;
    d = fraction2.integral;
    e = fraction2.numerator;
    f = fraction2.denominator;
    g = fraction_sum.integral;
    h = fraction_sum.numerator;
    i = fraction_sum.denominator;

    b = (a * c + b) * f;
    cout << b << endl;
    e = (d * f + e) * c;
    cout << e << endl;
    i = c*f;
    cout << i << endl;

    g = (b + e) % i;
    cout << g << endl;

    h = (b + e) / i;
    cout << h << endl;

    h = h/gcd(h,i);
    i=i/gcd(h,i);

    fraction_sum.integral = g;
    fraction_sum.numerator = h;
    fraction_sum.denominator = i;
    cout << g << h << i << endl;
    return fraction_sum;
}

int main() {
    fraction sum, x, temp;
    int n = 0, i = 2;

    cout << "Enter number of fractions:" << endl;
    cin >> n;

    cout << "Enter fraction 1:" << endl;
    temp.read();

    while (i <= n) {
        cout << "Enter fraction " << i << ":" << endl;
        x.read();
        sum = x + temp;
        temp = sum;
        i++;
    }

    sum.print();
    return 0;
}

Recommended Answers

All 2 Replies

To be more specific, I have problems with friend operator +.

The return of b, e, i are 0 only

g, h cannot be generated. The error msg is core dumped.

How can I fix them?

You unfortunately have several problems: So i am going to give you a quick summary of the problem and why I think you have made the mistake.

(i) In the constructor fraction::fraction(), you have written integral, numerator, denominator = 0; This DOES NOT do what you think, it does NOT set integral or numerator. If you wanted to do that then you should have done integral=numerator=denominator=0; . Much much better would have been to use an initializer list e.g. fraction::fraction() : integral(0),numerator(0),denominator(0) {} .
Note: Almost all modern compilers would have given you a warning about that if you had turned your warnings on. No beginner should ever compile without warnings on.

(ii) Input is not actually wrong but so ugly, surely improper fractions would be ok?
What about reading a line of input and then splitting it based on e.g. 4/5 or 3 4/5

(iii) friend fraction operator+ Never make something a friend that doesn't need to be. This just doesn't need to be a friend. It should also have this form: fraction operator+(const fraction&) const; Then it is (a) more efficient (b) much less likely to go wrong.

(iv) gcd and gcdr should be one function and not call each other backwards and forwards, or gcd_prep should call gcd (which would be the recursive function) once only [to allow the test for negative to the outside.]

(v) operator+ is written in such a way that I think you did not really work out what is going on, on paper first. Several things are going on, and mistakes are made.
-- First separate the optimization of the fraction, make the whole conversion to the simplest possible fraction in a separate method e.g. const fraction& simplify(); . Write that function so that if you give it a fraction 3 14/4 it can make it 6 1/2.
-- Second code the addition in a simpler way:

fraction operator+(const fraction& A)
{
   fraction Out;
   Out.integral=integrat+A.integral;     // Add the integrals
   // Put checks in here for zero denominator:
   Out.denominator=A.denominator*denominator;
   Out.numerator=A.numerator*denominator+ numerator*A.denominator;
   Out.simplify();
   return Out;
}

The actual mistake that you made in your code was doing this:
You call the function as this

fraction operator + (fraction, fraction) 
{
  //THESE are uninitialized objects:
  fraction fraction1, fraction2, fraction_sum;

and you have uninitialized objects:
You would have wanted :

fraction operator+ (fraction fraction1, fraction fraction2) 
{
  fraction fraction_sum;

However, AGAIN note the poor choice of name fraction1 is a typo away from fraction, and that is going to lead to HORRIBLE debugging problems. Never use a variable name that is less than three letters away from a class name in the usage scope. Note: fraction_sum is ONE letter away since fraction _sum very easy to type.


Finally h=h/gcd(h,i); i=i/gcd(h,i); which to work requires the OLD h not the new one. However, please do not EVER use a set of temporary names that meaningless, had you written int aI, aD, aN; etc it would have been million times easier to read. [Although I still think that is a poor choice of names]


Sorry for the long long list, but it is MUCH better to do this wrong once, since many of these ideas follow through in many many languages, and they certainly will effect every one of you C++ programs.

Be a part of the DaniWeb community

We're a friendly, industry-focused community of developers, IT pros, digital marketers, and technology enthusiasts meeting, networking, learning, and sharing knowledge.