Seeking advice on rational number class

[kenken]

Hello guys. I am trying to define a rational number class. I've written part of the class in the following code but the code seems hard to follow. I looking for advice and tips in this regard. Thanks in advance.

  #ifndef RATIONALS_H_INCLUDED
#define RATIONALS_H_INCLUDED

class Rationals
{
private:
    int num; // numerator
    int denom; // denominator
    int gcd(const int a, const int b); // Returns the greatest common divisor of two numbers
    void simplify(Rationals &obj); // Reduce the fraction to simplest form
public:
    Rationals(); // Default constructor. Initialize numerator to 1 and denominator to 1
    Rationals(int number); // Overloaded constructor. Initialize numerator to number and denominator to 1
    Rationals(int top, int bottom); // Overloaded constructor. Initialize numerator to top and denominator to bottom
    void print(); // Print out a Rationals object in the form of a/b, where a and b are integers.
    Rationals operator+(const Rationals &another); // Operator + is overloaded to allow addition of Rationals objects. Returns a new instance of Rationals
};



#endif // RATIONALS_H_INCLUDED


#include <iostream>
#include "Rationals.h"

//Default constructor. Initialize numerator to 0 and denominator to 1
Rationals::Rationals(): num(0), denom(1){}

// Overloaded constructor. Initialize numerator to number and denominator to 1
Rationals::Rationals(int number): num(number), denom(1){}

// Overloaded constructor. Initialize numerator to top and denominator to bottom
Rationals::Rationals(int top, int bottom): num(top), denom(bottom){}

// Prints out a Rationals object in the form of a/b, where a and b are integers
void Rationals::print() {std::cout << num << "/" << denom << std::endl;}

// Returns the greatest common divisor of two integers
int Rationals::gcd(const int a, const int b)
{
    if(b == 0)
        return a;
    else
        return gcd(b,a%b);

}

// Reduce the fraction to simplest form
void Rationals::simplify(Rationals &obj)
{
    int divisor = gcd(obj.num, obj.denom);
    obj.num /= divisor;
    obj.denom /= divisor;
}

// Operator + is overloaded to allow addition of Rationals objects. Returns a new instance of Rationals
Rationals Rationals::operator+(const Rationals &another)
{
    Rationals temp;

    temp.num = (num * another.denom) + (denom * another.num);
    temp.denom = denom * another.denom;
    simplify(temp);

    return temp;
}

[jonnin]

FYI gcd is part of C++'s built in tools. No need to rewrite it.

what seems hard to follow? It looks rather clean to me (somewhat incomplete, but clean).

[dhayden]

I don't think your code is hard to follow. It's actually pretty good so far. Still, I have a few comments:

simplify() should modify *this rather than taking a parameter.

There's no reason to have a this pointer when calling gcd(), so it should be a static member, or (better in my opinion) a static function in rational.cpp:

simplify() should handle negative numbers and should ensure that the denominator is always positive.

Make these changes and then write a main() program to test what you have:
- Create Rationals using each constructor and print it.
- Create some Rationals that need to be simplified, simplify them and print them.
- Add some rationals and test the result.

You will find more problems when you test. I'm deliberately not mentioning them so you can find and fix them yourself. If you get stuck for more than an hour, post your code and describe the problem.

 #ifndef RATIONALS_H_INCLUDED
#define RATIONALS_H_INCLUDED

class Rationals
{
private:
    int num; // numerator
    int denom; // denominator
    void simplify(); // Reduce the fraction to simplest form
public:
    Rationals(); // Default constructor. Initialize numerator to 1 and denominator to 1
    Rationals(int number); // Overloaded constructor. Initialize numerator to number and denominator to 1
    Rationals(int top, int bottom); // Overloaded constructor. Initialize numerator to top and denominator to bottom
    void print(); // Print out a Rationals object in the form of a/b, where a and b are integers.
    Rationals operator+(const Rationals &another); // Operator + is overloaded to allow addition of Rationals objects. Returns a new instance of Rationals
};



#endif // RATIONALS_H_INCLUDED


#include <iostream>
#include "Rationals.h"

//Default constructor. Initialize numerator to 0 and denominator to 1
Rationals::Rationals(): num(0), denom(1){}

// Overloaded constructor. Initialize numerator to number and denominator to 1
Rationals::Rationals(int number): num(number), denom(1){}

// Overloaded constructor. Initialize numerator to top and denominator to bottom
Rationals::Rationals(int top, int bottom): num(top), denom(bottom){}

// Prints out a Rationals object in the form of a/b, where a and b are integers
void Rationals::print() {std::cout << num << "/" << denom << std::endl;}

// Returns the greatest common divisor of two integers
static int gcd(const int a, const int b)
{
    if(b == 0)
        return a;
    else
        return gcd(b,a%b);

}

// Reduce the fraction to simplest form
void Rationals::simplify()
{
    // ensure denominator is positive
    if (denom < 0) {
	num = -num;
	denom = -denom;
    }
    
    int divisor = gcd(abs(num), denom);
    num /= divisor;
    denom /= divisor;
}

// Operator + is overloaded to allow addition of Rationals objects. Returns a new instance of Rationals
Rationals Rationals::operator+(const Rationals &another)
{
    Rationals temp;

    temp.num = (num * another.denom) + (denom * another.num);
    temp.denom = denom * another.denom;
    temp.simplify();

    return temp;
}

[Duthomhas]

+1 for everything said.

Just a couple thoughts for the future.


Commentary

I know excess commentary is useful, especially for a beginner.
As you move forward, though, you should look to reducing your commentary.

C++ code comes in two parts:

  • the interface (.h and .hpp files)
  • the implementation (.cpp files)

What this means is that for every module (like your "Rationals") you should have two files open when you are working on it: the header and the implementation.

All the commentary about using the class belongs in the header.

Any commentary in the implementation should be restricted to technical details that no one needs to know in order to use the class.


Don’t repeat yourself.

If you do, you will wind up doing what you did on lines 12 and 27. Oops!
With that in mind, your class declaration (in the header) can be easily made to read like this:

class Rationals
{
private:
    int num;
    int denom;
    void simplify();
public:
    Rationals(); // --> 0/1
    Rationals(int number); // --> number/1
    Rationals(int numerator, int denominator);
    void print(); // Print to standard output as, for example, "0/1"
    Rationals operator+(const Rationals &another); 
};

Notice that things like “simplify” do not need commentary, as the concept of “simplifying a fraction” is well-known and understood; unless it is doing something different (which it shouldn’t be), then no explanation is necessary for anyone who has passed 5^th grade.

Thereafter, the only commentary in the implementation should be stuff that you don’t need to know to use the class. For example, dhayden’s version of simplify() has nice, useful commentary inside the function to explain what the code is doing. Line 48 can totally disappear.

Mind you, this is different than a = b; // set a equal to b . Remember, commentary is not meant to repeat information that can be obtained just by glancing at the code. It is to explain, in general terms, why the code is doing what it does, and any non-obvious details. dhayden did this with his comment: it summarized a weird collection of negations to make things obvious.



Names Of Things

What you name things makes a huge difference in understanding. Your code is simple enough that there is no difficulty in understanding everything, but it can still be improved.

So far, you have used three different ways to refer to the components of a fraction:

  • num / denom
  • top / bottom
  • a / b

They all work, but you should choose one and stick with it. Consistency matters.

Rationals(int num, int denom); Rationals(int num, int denom): num(num), denom(denom) {}


Likewise, the word “Rationals” is plural, implying multiple rational numbers. But the class is for a single rational number. The fix is easy:

    class Rational

Note the lack of an s: this is a single rational number!
I am aware that you may not have control over the name of your class when doing homework. Just keep stuff like this in mind when naming things.


Readability

Your code is already very clean. But as you have observed, there is more to readability than just clean code. The way you present it makes a difference.

tl;dr:

  • Don’t repeat yourself
  • Be consistent
  • Information for users → header file
  • Information about the implementation → with the code in the .cpp file

Hope this helps. :O)

[kenken]

Thank yo so much guys! Your inputs and helps are very valuable for me because I am picking up C++ on my own. Taking your combined suggestions, i modified my Rationals class as followed. I made gdc() a static function since there's no need for each object to have its own gcd(). I also add a function named top_negative() to ensure that the negative sign is associated with the numerator. Lastly, I make use of the "this operator" instead of passing an object into simpify(). I question jumped at me as I made the changes. The question is, Is is safe for function parameters to have the same name as the class variables. For instance i had

Rationals::Ractionals(int num, int denom): num(num), denom(denom){}. I assume the compiler can differentiate the nums and denoms because it didn't give me any issues.

  #ifndef RATIONALS_H_INCLUDED
#define RATIONALS_H_INCLUDED

class Rationals
{
private:
    int num; // numerator
    int denom; // denominator
    static int gcd(const int num, const int denom); // Returns GCD of (a,b)
    void simplify(); // 2/4 -> 1/2
    void top_negative(); // negative sign associated with numerator
public:
    Rationals(); // 0/1
    Rationals(int num); // number/1
    Rationals(int num, int denom); // top/bottom
    void print(); // print num/denom
    Rationals operator+(const Rationals &another); //Returns a new instance of Rationals
};



#endif // RATIONALS_H_INCLUDED


#include <iostream>
#include "Rationals.h"

Rationals::Rationals(): num(0), denom(1){}

Rationals::Rationals(int num): num(num), denom(1){}

Rationals::Rationals(int num, int denom): num(num), denom(denom){}

void Rationals::print() {std::cout << num << "/" << denom << std::endl;}

int Rationals::gcd(const int num, const int denom)
{
    if(denom == 0)
        return num;
    else
        return gcd(denom,num%denom);

}

void Rationals::top_negative()
{
    if(this-> denom < 0)
    {
        (this-> num) *= -1;
        (this-> denom) *= -1;
    }
}

void Rationals::simplify()
{
    int divisor = gcd(this -> num, this -> denom);
    (*this).num /= divisor;
    (*this).denom /= divisor;
}

Rationals Rationals::operator+(const Rationals &another)
{
    Rationals temp;

    temp.num = (this -> num * another.denom) + (this -> denom * another.num);
    temp.denom = (this -> denom) * another.denom;
    temp.simplify();
    temp.top_negative();

    return temp;
}


#include <iostream>
#include "Rationals.h"

using namespace std;

int main()
{
  Rationals rat[] ={4,-4,Rationals(2,4),Rationals(-2,4),Rationals(2,-3),Rationals(-2,-4)};
  for(int i = 0; i < 6; i++)
  rat[i].print();

  cout << endl;
  Rationals r(3,-2);
  Rationals r1(1,2);
  (r+r1).print();

}

[Duthomhas]

Suggestions:

The denominator should never be negative: simplify() should make sure of that.
The implementation does not need a function to test a single field. Just write if (num < 0) where necessary. Only add functions where potential abstraction is necessary.

If you want your users to be able to directly test for negative rationals, then provide it as a public member function. You might call it “is_negative()”.

That said, you can easily supply a comparison operator to do this for you:

bool Rationals::operator < ( const Rationals& rhs ) const
{
  return (num * rhs.denom) < (rhs.num * denom);
}

Since your int → Rationals constructor is not marked explicit, this should be able to handle comparison with plain-old integers as well. (I might have forgotten some caveats in there. Since you only work on int, comparisons with long long for example, or doubles won’t work — you’ll need to provide a conversion ctor and/or comparitor for them as well if you intend to permit it.)

Now your users can test for negativity the same way they do with all other numeric objects:

Rationals x{ -7 };
if (x < 0)
  std::cout << "yeah!\n";
else
  std::cout << "fooey.\n";

Functions that do not modify the class data should be marked const. This significantly increases the places you can use a Rationals.

class Rationals
{
private:
    int num;
    int denom;
    static int gcd(const int num, const int denom); // Returns GCD of (a,b)
    void simplify(); // 2/4 -> 1/2
public:
    Rationals(); // 0/1
    Rationals(int num); // number/1
    Rationals(int num, int denom); // top/bottom
    void print() const; // print num/denom
    void is_negative() const;
    Rationals operator+(const Rationals &another) const;
    bool operator<(const Rationals& rhs) const;
};

Etc.

Good job!

[edit]
BTW, the C++ <numeric> library provides functions you may find use for, like gcd() and lcm(). https://en.cppreference.com/w/cpp/header/numeric