-1
\$\begingroup\$

This fraction class is a project I'm working on for fun. I want it to the best it can be. Please let me know if I'm following best procedures or if I'm missing something.

**Update So, I took the advice from the answers I received and tried to improve my code. I'm curious, can I improve more? Am I missing anything one would consider important in such an object, maybe an overload of operator++ ? This post has been updated with the improved fraction class code below the original, "stdafx.h" remains unchanged.

The "stdafx.h" file below

#pragma once

#include "targetver.h"

#include <stdio.h>
#include <tchar.h>
#include <iostream>
#include <limits>
#include <string>
#include <sstream>
#include "fraction.h"
#include <cmath>

The "fraction.h" file below.

#pragma once

class fraction
{
    private:
        unsigned int numerator;
        unsigned int denominator;
        char fraction_symbol;
        bool is_reduced;
        bool is_neg;

    public:
        fraction();
        fraction(int, int);
        fraction(unsigned int);
        fraction(int);
        fraction(float);
        fraction(double);
        fraction reduce();
        int unsigned get_numerator();
        int unsigned get_denominator();
        void set_numerator(int);
        void set_denominator(int);
        void set_neg();
        bool check_neg();
        friend std::istream& operator >> (std::istream & is, fraction & p);
        friend std::ostream& operator << (std::ostream & os, fraction & p); 
        friend fraction operator+ (fraction, fraction);
        friend fraction operator- (fraction, fraction);
        friend fraction operator* (fraction, fraction);
        friend fraction operator/ (fraction, fraction);
        friend bool operator== (fraction, fraction);
        friend bool operator!= (fraction, fraction);
        friend bool operator<= (fraction, fraction);
        friend bool operator>= (fraction, fraction);
        friend bool operator< (fraction, fraction);
        friend bool operator> (fraction, fraction);
};

The "fraction.cpp" file below.

 #include "stdafx.h"

 static int gcd(int, int);      /// barrowed function see definition.
 static void input_check(std::string&);


fraction::fraction()
{
    fraction_symbol = '/';
    is_reduced = false;
    is_neg = false;
    numerator = 1;
    denominator = 1;
}

fraction::fraction(int numer,int denom)
{
    if (denom == 0)
    {
         while (std::cout << "Illegal! Division by 0!" << std::endl << 
         "Please 
         enter a new denominator: " && !(std::cin >> denom) || (denom == 0))
         {
             std::cin.clear();
             std::cin.ignore(std::numeric_limits<std::streamsize>::max(), 
             '\n');
             std::cout << "Invalid entry, please try again." << std::endl;
        }
    }
    if ((numer < 0) && (denom < 0))
    {
        numer *= -1;
        denom *= -1;
    }
    if (numer < 0 )
    {
        is_neg = true;
        numer *= -1;
    }
    if (denom < 0)
    {
        is_neg = true;
        denom *= -1;
    }
    numerator = numer;
    denominator = denom;
    fraction_symbol = '/';
}

fraction::fraction(unsigned int temp)
{
    numerator = temp;
    denominator = temp;
    is_neg = false;
    reduce();
}

fraction::fraction( int temp)
{
    if (temp < 0)
    {
        is_neg = true;
        temp *= -1;
    }
    else
    {
        is_neg = false;
    }
    numerator = temp;
    denominator = temp;
    reduce();
}

int gcd(int a, int b)
{
    if (a == 0)
    {
        return b;
    }
    else if (b == 0)
    {   
        return a;
    }
    if (a < b)
    {           
        return gcd(a, b % a);
    }
    else
    {   
        return gcd(b, a % b);
    }
}


fraction::fraction(float temp)
{
    if (temp < 0)
    {
        is_neg = true;
        temp *= -1;
    }
    else
    {
        is_neg = false;
    }
    double integral = std::floor(temp);                                                    
    ///Euclidean algorithm , from here to
    double frac = temp - integral;
    //possible tweak could be done here, This is the accuracy below.                           
    const unsigned int precision = 10000;               
    unsigned int gcd_ = unsigned int(gcd(int(round(frac * precision)), 
    int(precision)));   
    /* stackoverflow.com, user: qbt937, url: 
    https://stackoverflow.com/questions/26643695/converting-decimal-to-
    fraction-c */
    denominator = precision / gcd_;
    numerator = unsigned int(round(frac * precision) / gcd_);                              
    //here. credit for code block in middle...
    reduce();
    numerator += unsigned int((integral*denominator));
    reduce();
}

fraction::fraction(double temp)
{
    if (temp < 0)
    {
        is_neg = true;
        temp *= -1;
    }
    else
    {
        is_neg = false;
    }
    double integral = std::floor(temp);                                                    
    ///Euclidean algorithm , from here to
    double frac = temp - integral;
    const unsigned int precision = 10000;                                                  
    //possible tweak could be done here, This is the accuracy.
    unsigned int gcd_ = unsigned int(gcd(int(round(frac * precision)), 
    int(precision)));   
    /* stackoverflow.com, user: qbt937, url: 
    https://stackoverflow.com/questions/26643695/converting-decimal-to-
    fraction-c */
    denominator = precision / gcd_;
    numerator = unsigned int(round(frac * precision) / gcd_);                              
    //here. credit for code block in middle...
    reduce();
    numerator += unsigned int((integral*denominator));
    reduce();
}

fraction fraction::reduce()
{
    for (long int i = (denominator * numerator);i > 1; i--)
    {
        if ((denominator % i == 0) && ((numerator % i) == 0))
        {
            is_reduced = true;
            numerator /= i;
            denominator /= i;
        }
    }
    if (is_reduced == false)
    {
        std::cout << "This fraction could not be reduced." << std::endl;
    }
    return *this;
}

unsigned int fraction::get_numerator()
{
    return numerator;
}

unsigned int fraction::get_denominator() 
{
    return denominator;
}

void fraction::set_numerator(int temp)
{
    if (temp < 0)
    {
        set_neg();
        temp *= -1;
    }
    else
    {
        numerator = temp;
    }  

}

void fraction::set_denominator(int temp)
{
    if (temp == 0)
    {
        while (std::cout << "Illegal! Division by 0!" << std::endl << 
        "Please enter a new denominator: " && !(std::cin >> temp) || (temp 
        == 0))   ///edit here to work in visual studios atleast....
            {
                std::cin.clear();
                std::cin.ignore(std::numeric_limits<std::streamsize>::max(), 
                '\n');
                std::cout << "Invalid entry, please try again." << 
                std::endl;
            }
    }
    if (temp < 0)
    {
        set_neg();
        temp *= -1;
        denominator = temp;
    }
    else
    {
        denominator = temp;
    }
}

std::istream& operator >> (std::istream & is, fraction & p)
{
    std::string temp_input;
    if (char(is.peek()) == '\n')
    {
        is.ignore();
    }
    std::getline(is, temp_input);
    input_check(temp_input);
    std::istringstream temp_string(temp_input);
    char holder = ' ';
    if (temp_input[0] == '-')
    {
        p.is_neg = true;
        temp_string >> holder >> p.numerator >> holder >> p.denominator;
    }
    else
    {
        temp_string >> p.numerator >> holder >> p.denominator;
    }
    return is;
}

std::ostream &operator << (std::ostream & os, fraction & p)
{
    if (p.is_neg == true)
    {
        os << "-" << p.numerator << "/" << p.denominator << std::flush;
        return os;
    }
    else
    {
        os << p.numerator << "/" << p.denominator << std::flush;
        return os;
    }
}


void input_check(std::string &temp_input)
{
    bool error = false;
    bool is_negative = false;
    int frac_symb_index = -1;   
    if (temp_input.length() < 3)
    {
        std::cout << "Invalid entry, please try again: " << std::flush;
        if (char(std::cin.peek()) == '\n')
        {
            std::cin.ignore();
        }
        std::getline(std::cin, temp_input);
        input_check(temp_input);
    }
    switch (temp_input[0])
    {
        case '-':
            is_negative = true;
            if (temp_input[1] == '/')
            {
                error = true;
            }
            break;
        case '0':
        case '1':
        case '2':
        case '3':
        case '4':
        case '5':
        case '6':
        case '7': 
        case '8':
        case '9':
            break;

        default:
            error = true;
            break;
    }
    int i;
    if (is_negative == true)
    {
        i = 1;
    }
    else
    {
        i = 0;
    }
    for (; i < int(temp_input.length() - 1); i++)
    {
        switch (temp_input[i])
        {
            case '0':
            case '1':
            case '2':
            case '3':
            case '4':
            case '5':
            case '6':   
            case '7':
            case '8':
            case '9':   
                break;
            case '/':
                frac_symb_index = i;
                break;  

            default:
                error = true;
                break;
        }
    }
    if (is_negative == true)
    {
        i = 1;
    }
    else
    {
        i = 0;
    }
    for(; i < frac_symb_index; i++)
    {
        switch (temp_input[i])
        {
            case '0':
            case '1':
            case '2':
            case '3':
            case '4':
            case '5':
            case '6':
            case '7':
            case '8':
            case '9':
                break; 

            default:
                error = true;
                break;
        }
    }
    for (int j = (temp_input.length()-1); frac_symb_index < j; j--)
    {
        switch (temp_input[j])
        {
            case '0':
            case '1':
            case '2':
            case '3':
            case '4':
            case '5':
            case '6':
            case '7':
            case '8':
            case '9':
                break; 

            default:
                error = true;
                break;
        }
    }
    if (error == true)
    {
        std::cout << "Invalid entry, please try again: " << std::flush;
        if (char(std::cin.peek()) == '\n')
        {
            std::cin.ignore();
        }
        std::getline(std::cin, temp_input);
        input_check(temp_input);
    }
}

fraction operator+ (fraction one, fraction two)  
{
    int added_num = 0;
    int common_denom = 0;
    bool is_it_neg = false;
    if (one.is_neg == false && two.is_neg == false)
    {
        added_num = ((one.get_numerator() * two.get_denominator()) + 
        (two.get_numerator() * one.get_denominator()));
        common_denom = (one.get_denominator() * two.get_denominator());
    }
    if (one.is_neg == true && two.is_neg == true )
    {
        added_num = ((one.get_numerator() * two.get_denominator()) + 
        (two.get_numerator() * one.get_denominator()));
        common_denom = (one.get_denominator() * two.get_denominator());
        is_it_neg = true;
    }
    if (one.is_neg == true && two.is_neg == false)
    {
        added_num = ((one.get_numerator() * two.get_denominator()) - 
        (two.get_numerator() * one.get_denominator()));
        common_denom = (one.get_denominator() * two.get_denominator());
        if (added_num > 0)
        {
            is_it_neg = true;
        }
    }
    if (one.is_neg == false && two.is_neg == true)
    {
        added_num = ((one.get_numerator() * two.get_denominator()) - 
        (two.get_numerator() * one.get_denominator()));
        common_denom = (one.get_denominator() * two.get_denominator());
        if (added_num < 0)
        {
            added_num *= -1;
            is_it_neg = true;
        }
    }
    fraction result(added_num,common_denom);
    if (is_it_neg == true)
    {
        result.set_neg();
    }
    result.reduce();
    return result;
}

void fraction::set_neg()
{
    if (is_neg == true)
    {
        is_neg = false;
    }
    else
    {
        is_neg = true;
    }
}

bool fraction::check_neg()
{
    if (is_neg == true)
    {
        return true;
    }
    else
    {
        return false;
    }
}

fraction operator- (fraction one, fraction two)
{
    int subed_num = 0;
    int common_denom = 0;
    bool is_it_neg = false;
    if (one.is_neg == false && two.is_neg == false)
    {
        subed_num = ((one.get_numerator() * two.get_denominator()) - 
        (two.get_numerator() * one.get_denominator()));
        common_denom = (one.get_denominator() * two.get_denominator());
        if (subed_num < 0)
        {
            subed_num *= -1;
            is_it_neg = true;
        }
    }
    if (one.is_neg == true && two.is_neg == true)
    {
        subed_num = (((one.get_numerator() * -1) * two.get_denominator()) + 
        (two.get_numerator() * one.get_denominator()));
        common_denom = (one.get_denominator() * two.get_denominator());
        if (subed_num < 0)
        {
            subed_num *= -1;
            is_it_neg = true;
        }
    }
    if (one.is_neg == true && two.is_neg == false)
    {
        subed_num = ((one.get_numerator() * two.get_denominator()) + 
        (two.get_numerator() * one.get_denominator()));
        common_denom = (one.get_denominator() * two.get_denominator());
        is_it_neg = true;
    }
    if (one.is_neg == false && two.is_neg == true)
    {
        subed_num = ((one.get_numerator() * two.get_denominator()) + 
        (two.get_numerator() * one.get_denominator()));
        common_denom = (one.get_denominator() * two.get_denominator());
    }
    char temp = '+';
    if (is_it_neg == true)
    {
        temp = '-';
    }
    fraction result(subed_num, common_denom);
    if (is_it_neg == true)
    {
        result.set_neg();
    }
    result.reduce();
    return result;
}

fraction operator* (fraction a, fraction b)
{
    fraction result((a.get_numerator() * b.get_numerator()),
    (a.get_denominator() * b.get_denominator()));
    if (a.check_neg() || b.check_neg())
    {
        result.set_neg();
    }
    result.reduce();
    return result;
}

fraction operator/ (fraction a, fraction b)
{
    fraction result((a.get_numerator() * b.get_denominator()), 
    (a.get_denominator() * b.get_numerator()));
    if (a.check_neg() || b.check_neg())
    {
        result.set_neg();
    }
    result.reduce();
    return result;
}

bool operator== (fraction a, fraction b)
{
    double c = (double(a.get_numerator()) / double(a.get_denominator()));
    double d = (double(b.get_numerator()) / double(b.get_denominator()));
    if (a.check_neg())
    {
        c *= -1;
    }
    if (b.check_neg())
    {
        d *= -1;
    }
    if (c == d)
    {
        return true;
    }
    else
    {
        return false;
    }
}

bool operator!= (fraction a, fraction b)
{
    double x = (double(a.get_numerator() )/ double(a.get_denominator()));
    double y = (double(b.get_numerator()) / double(b.get_denominator()));
    if (a.check_neg())
    {
        x *= -1;
    }
    if (b.check_neg())
    {
        y *= -1;
    }
    if (x != y)
    {
        return true;
    }
    else
    {
        return false;
    }
}

bool operator<= (fraction a, fraction b)
{
    double e = (double(a.get_numerator()) / double(a.get_denominator()));
    double f = (double(b.get_numerator()) / double(b.get_denominator()));
    if (a.check_neg())
    {
        e *= -1;
    }
    if (b.check_neg())
    {
        f *= -1;
    }
    if (e <= f)
    {
        return true;
    }
    else
    {
        return false;
    }
}

bool operator>= (fraction a, fraction b)
{
    double g = (double(a.get_numerator()) / double(a.get_denominator()));
    double h = (double(b.get_numerator()) / double(b.get_denominator()));
    if (a.check_neg())
    {
        g *= -1;
    }
    if (b.check_neg())
    {
        h *= -1;
    }
    if (g >= h)
    {
        return true;
    }
    else
    {
        return false;
    }
}

bool operator< (fraction a, fraction b) 
{   
    double i = (double(a.get_numerator()) / double(a.get_denominator()));
    double j = (double(b.get_numerator()) / double(b.get_denominator()));
    if (a.check_neg())
    {
        i *= -1;
    }
    if (b.check_neg())
    {
        j *= -1;
    }
    if (i < j)
    {
        return true;
    }
    else
    {
        return false;
    }
}

bool operator> (fraction a, fraction b)
{
    double k = (double(a.get_numerator()) / double(a.get_denominator()));
    double l = (double(b.get_numerator()) / double(b.get_denominator()));
    if (a.check_neg())
    {
        k *= -1;
    }
    if (b.check_neg())
    {
        l *= -1;
    }
    if (k > l)
    {
        return true;
    }
    else
    {
        return false;
    }
}

**updated code below "fraction.h"

#pragma once

class fraction
{
    private:
        unsigned int numerator;       
        unsigned int denominator;     
        bool is_reduced;
        bool is_neg;
        void toggle_neg();
        void reduce();

    public:
        fraction();
        fraction(int, int);
        fraction(unsigned int);
        fraction(int);
        fraction(float);
        fraction(double);
        friend std::istream &operator >> (std::istream &is, fraction &p);
        friend std::ostream &operator << (std::ostream &os, fraction &p); 
        int& operator=(fraction const &);
        fraction& operator += (fraction const &);   
        friend fraction operator + (fraction const&, fraction const&);   
        fraction& operator -= (fraction const &);
        friend fraction operator - (fraction, fraction);
        fraction& operator *= (fraction const &);
        friend fraction operator * (fraction, fraction);
        fraction& operator /= (fraction const & two);
        friend fraction operator / (fraction, fraction);
        bool operator == (fraction const&) const;
        bool operator != (fraction const&) const;
        bool operator < (fraction const &) const;
        bool operator > (fraction const &) const;
        bool operator <= (fraction const &) const;
        bool operator >= (fraction const &) const;
};

**updated code below "fraction.cpp"

#include "stdafx.h"


static int gcd(int, int);              /// barrowed function see definition.
static void input_check(std::string&);


int gcd(int a, int b)     
{
    if (a == 0)
    {
        return b;
    }
    else if (b == 0)
    {
         return a;
    }
    if (a < b)
    {
        return gcd(a, b % a);
    }
    else
    {
        return gcd(b, a % b);
    }
}              

void input_check(std::string &temp_input)
{
    bool error = false;
    bool is_negative = false;
    int frac_symb_index = -1;
    if (temp_input.length() < 3)
    {
        std::cout << "Invalid entry, please try again: ";
        if (char(std::cin.peek()) == '\n')
        {
            std::cin.ignore();
        }
        std::getline(std::cin, temp_input);
        input_check(temp_input);
    }
    switch (temp_input[0])
    {
        case '-':
            is_negative = true;
            if (temp_input[1] == '/')
            {
                error = true;
            }
            break;  
        case '0':
        case '1':
        case '2':
        case '3':
        case '4':
        case '5':
        case '6':
        case '7':
        case '8':
        case '9':
            break;

        default:
            error = true;
            break;
    }
    int i;
    if (is_negative)
    {
        i = 1;
    }
    else
    {
        i = 0;
    }
    for (; i < int(temp_input.length() - 1); i++)
    {
        switch (temp_input[i])
        {
            case '0':
            case '1':
            case '2':
            case '3':
            case '4':
            case '5':
            case '6':
            case '7':
            case '8':
            case '9':
                break;
            case '/':
                frac_symb_index = i;
                break;

            default:
                error = true;
                break;
        }
    }
    if (is_negative)
    {
        i = 1;
    }
    else
    {
        i = 0;
    }
    for (; i < frac_symb_index; i++)
    {
        switch (temp_input[i])
        {
            case '0':
            case '1':
            case '2':
            case '3':
            case '4':
            case '5':
            case '6':
            case '7':
            case '8':
            case '9':
                break;  

            default:
                error = true;
                break;
        }
    }
    for (int j = (temp_input.length() - 1); frac_symb_index < j; j--)
    {
        switch (temp_input[j])
        {
            case '0':
            case '1':
            case '2':
            case '3':
            case '4':
            case '5':
            case '6':
            case '7':
            case '8':
            case '9':
                break; 

            default:
                error = true;
                break;
        }
    }
    if (error)
    {
        std::cout << "Invalid entry, please try again: ";
        if (char(std::cin.peek()) == '\n')
        {
            std::cin.ignore();
        }
        std::getline(std::cin, temp_input);
        input_check(temp_input);}
}

void fraction::toggle_neg()
{
    (is_neg) ? is_neg = false : is_neg = true;
}

void fraction::reduce()
{
    int max = 0;
    (this->numerator >= this->denominator) ? max = numerator : max = 
    denominator;
    for (long int i = max; i > 1; i--)
    {
        if ((this->denominator % i == 0) && ((this->numerator % i) == 0))
        {
            this->is_reduced = true;
            this->numerator /= i;
            this->denominator /= i;
        }
    }
}

fraction::fraction()
    :is_reduced(false)
    , is_neg(false)
    , numerator(0)
    , denominator(0)
{

}

fraction::fraction(int numer, int denom)
    :fraction()
{
    if (denom == 0)
    {
        std::cout << "Illegal division by zero."///error handling on the way
    }
    if ((numer < 0) && (denom < 0))
    {
        numer *= -1;
        denom *= -1;
    }
    if (numer < 0 )
    {
        is_neg = true;
        numer *= -1;
    }
    if (denom < 0)
    {
        is_neg = true;
        denom *= -1;
    }
    this->numerator = numer;
    this->denominator = denom;  
    this->reduce();
}

fraction::fraction(unsigned int temp)
    :fraction()
{
    numerator = temp;
    denominator = 1;
}

fraction::fraction(int temp)
    :fraction()
{
    if (temp < 0)
    {
        is_neg = true;
        temp *= -1;
    }
    else
    {
        is_neg = false;
    }
    numerator = temp;
    denominator = temp;
    reduce();
}

fraction::fraction(float temp)
{
    if (temp < 0)
    {
        is_neg = true;
        temp *= -1;
    }
    else
    {
        is_neg = false;
    }
    double integral = std::floor(temp);                                                    
    ///Euclidean algorithm , from here to
    double frac = temp - integral;
    const unsigned int precision = 1000000;                                                  
    ///possible tweak could be done here, This is the accuracy.
    unsigned int gcd_ = unsigned int(gcd(int(round(frac * precision)), 
    int(precision)));   // stackoverflow.com, user: qbt937, url: 
    https://stackoverflow.com/questions/26643695/converting-decimal-to-
    fraction-c
    denominator = precision / gcd_;
    numerator = unsigned int(round(frac * precision) / gcd_);                              
    //here. credit for code block in middle...
    reduce();
    numerator += unsigned int((integral*denominator));
    reduce();
}

fraction::fraction(double temp)
    :fraction()
{
    if (temp < 0)
    {
        is_neg = true;
        temp *= -1;
    }
    else
    {
        is_neg = false;
    }
    double integral = std::floor(temp);                                                    
    ///Euclidean algorithm , from here to
    double frac = temp - integral;
    const unsigned int precision = 10000;                                                  
    ///possible tweak could be done here, This is the accuracy.
    unsigned int gcd_ = unsigned int(gcd(int(round(frac * precision)), 
    int(precision)));   // stackoverflow.com, user: qbt937, url: 
    https://stackoverflow.com/questions/26643695/converting-decimal-to-
    fraction-c
    denominator = precision / gcd_;
    numerator = unsigned int(round(frac * precision) / gcd_);                              
    //here. credit for code block in middle...
    reduce();
    numerator += unsigned int((integral*denominator));
    reduce();
}

std::istream &operator >> (std::istream &is, fraction &p)
{
    std::string temp_input;
    if (char(is.peek()) == '\n')
    {
        is.ignore();
    }
    std::getline(is, temp_input);
    input_check(temp_input);
    std::istringstream temp_string(temp_input);
    char holder = ' ';
    if (temp_input[0] == '-')
    {
        p.is_neg = true;
        temp_string >> holder >> p.numerator >> holder >> p.denominator;
    }
    else
    {
        temp_string >> p.numerator >> holder >> p.denominator;
    }
    return is;
}

std::ostream &operator << (std::ostream &os, fraction &p)
{
    if (p.is_neg)
    {
        os << "-" << p.numerator << "/" << p.denominator;
        return os;
    }
    else
    {
        os << p.numerator << "/" << p.denominator;
        return os;
    }
}

fraction& fraction::operator += (fraction const &two)
{
    if ((this->numerator * this->denominator) == 0)
    {
        this->numerator = two.numerator;
        this->denominator = two.denominator;
        return *this;
    }
    if ((two.numerator * two.denominator) == 0)
    {
        return *this;
    }
    bool is_it_neg = false;
    int added_num = 0;
    int common_denom = 0;
    int twonumerator = two.numerator;
    int twodenominator = two.denominator;
    if (!(this->is_neg) && !(two.is_neg))
    {
        added_num = ((this->numerator * two.denominator) + (two.numerator * 
        this->denominator));
        common_denom = (this->denominator * two.denominator);
    }
    if (this->is_neg && two.is_neg)
    {
        added_num = ((this->numerator * two.denominator) + (two.numerator * 
        this->denominator));
        common_denom = (this->denominator * two.denominator);
        is_it_neg = true;
    }
    if (this->is_neg && !(two.is_neg))
    {
        added_num = ((this->numerator * two.denominator) - (two.numerator * 
        this->denominator));
        common_denom = (this->denominator * two.denominator);
        if (added_num > 0)
        {
            is_it_neg = true;
        }
    }
    if (!(this->is_neg) && two.is_neg)
    {
        added_num = ((this->numerator * two.denominator - (two.denominator * 
        this->denominator)));
        common_denom = (this->denominator * two.denominator);
        if (added_num < 0)
        {
            added_num *= -1;
            is_it_neg = true;
        }
    }
    if(is_it_neg)
    {
        this->toggle_neg();   
    }
    this->numerator = added_num;
    this->denominator = common_denom;
    this->reduce();
    return *this;
}              

fraction operator + (fraction const &one, fraction const &two)
{
    fraction result(one);
    return (result += two);
}

fraction& fraction::operator -= (fraction const &two)
{
    int subed_num = 0;
    int common_denom = 0;
    bool is_it_neg = false;
    if ((this->numerator * this->denominator) == 0)
    {
        this->numerator = two.numerator;
        this->denominator = two.denominator;
        return *this;
    }
    if ((two.numerator * two.denominator) == 0)
    {
        return *this;
    }
    if (!(this->is_neg) && !(two.is_neg))
    {
        subed_num = ((this->numerator * two.denominator) - (two.numerator * 
        this->denominator));
        common_denom = (this->denominator * two.denominator);
        if (subed_num < 0)
        {
            subed_num *= -1;
            is_it_neg = true;
        }
    }
    if (this->is_neg && two.is_neg)
    {
        subed_num = (((this->numerator * -1) * two.denominator) + 
        (two.numerator 
        * this->denominator));
        common_denom = (this->denominator * two.denominator);
        if (subed_num < 0)
        {
            subed_num *= -1;
            is_it_neg = true;
        }
    }
    if (this->is_neg && !(two.is_neg))
    {
        subed_num = ((this->numerator * two.denominator) + (two.numerator * 
        this->denominator));
        common_denom = (this->denominator * two.denominator);
        is_it_neg = true;
    }
    if (!(this->is_neg) && two.is_neg)
    {
        subed_num = ((this->numerator * two.denominator) + (two.numerator * 
        this->denominator));
        common_denom = (this->denominator * two.denominator);
    }
    this->numerator = subed_num;
    this->denominator = common_denom;
    if (is_it_neg)
    {
        this->toggle_neg();
    }
    this->reduce();
    return *this;
}

fraction operator - (fraction one, fraction two)
{
    fraction result(one);
    return (result -= two);
}

fraction& fraction::operator *= (fraction const &two)
{
    if((this->is_neg || two.is_neg) && (!(this->is_neg) && !(two.is_neg)))
    {
        this->toggle_neg();
    }
    this->numerator = (this->numerator * two.numerator);
    this->denominator = (this->denominator * two.denominator);
    this->reduce();
    return *this;
}

fraction operator * (fraction a, fraction b)
{
    fraction result(a);
    return (result *= b);
} 

fraction& fraction::operator /= (fraction const & two)
{
    this->numerator = (this->numerator * two.denominator);
    this->denominator = (this->denominator * two.numerator);
    if ((this->is_neg || two.is_neg) && (!(this->is_neg) && !(two.is_neg)))
    {
        this->toggle_neg();
    }
    this->reduce();
    return *this;
}

fraction operator / (fraction a, fraction b)
{
    fraction result(a);
    return (result /= b);
}

bool fraction::operator == (fraction const &b) const
{
    double c = (double(this->numerator) / double(this->denominator));
    double d = (double(b.numerator) / double(b.denominator));
    if (this->is_neg)
    {
        c *= -1;
    }
    if (b.is_neg)
    {
        d *= -1;
    }
    return (c == d);
}

bool fraction::operator != (fraction const &b) const
{
    return ((*this == b) ? false : true);
}

bool fraction::operator < (fraction const &b) const
{
    double i = (double(this->numerator) / double(this->denominator));
    double j = (double(b.numerator) / double(b.denominator));
    if (this->is_neg)
    {
        i *= -1;
    }
    if (b.is_neg)
    {
        j *= -1;
    }
    return (i < j);
}

bool fraction::operator > (fraction const &b) const
{
    double k = (double(this->numerator) / double(this->denominator));
    double l = (double(b.numerator) / double(b.denominator));
    if (this->is_neg)
    {
        k *= -1;
    }
    if (b.is_neg)
    {
        l *= -1;
    }
    return (k > l);
}

bool fraction::operator <= (fraction const &b) const
{
    return ((*this < b) || (*this == b));
}

bool fraction::operator >= (fraction const &b) const
{
    return ((*this > b) || (*this == b));
}
\$\endgroup\$
  • \$\begingroup\$ Just to be clear...The gcd function was borrowed from stackoverflow.com user: qbt937. I must give credit where credit is due! \$\endgroup\$ – G. Foster Jul 27 '17 at 20:04
  • \$\begingroup\$ That cannot compile due to unbalanced }. \$\endgroup\$ – Deduplicator Jul 27 '17 at 20:33
  • \$\begingroup\$ Something is weird about your std::istream& operator >> implementation. Please ensure that you have posted your code correctly. Also, this is clearly not valid c code, so you should remove that tag. \$\endgroup\$ – 200_success Jul 27 '17 at 21:33
  • \$\begingroup\$ Code edited...Sorry about that. \$\endgroup\$ – G. Foster Jul 27 '17 at 21:56
  • \$\begingroup\$ This is still invalid c code, so I'm not reopening the question until you remove that tag. \$\endgroup\$ – 200_success Jul 27 '17 at 22:11
3
\$\begingroup\$

Re-Use of constructors

Apart from the problem Loki Astari mentioned with the user input in the constructor, your doing a lot of initialization logic multiple times and sometimes even forget some of it. For example the member fraction_symbol is only set in two constructors (and you don't use that one in your stream operator). You should redirect to your other constructors on only do the special initialization. Also it's better to use initializer lists than assignments:

fraction::fraction(int numer, int denom)
    : fraction_symbol('/')
    , is_reduced(false)
    , is_neg(false)
{
   //...
}

fraction::fraction()
    : fraction(0, 1)
{
}

Simplications

No need to check if (value) return true; else return false; Simply return the value.

bool fraction::check_neg()
{
    return is_neg;
}

The same goes for set_neg():

void fraction::set_neg()
{
    is_neg = !is_neg;
}

Also it should be renamed to something else. A set method implies that the user can actually define what the value is. But this method simply toggles the value.

Keep it DRY

As I said in the first section, you are repeating a lot of logic again and again. Try to reuse your previous logic in other similar code blocks. For example the comparison operator are all pretty much the same. What you really only need it the == and the < operator. All the other comparison operators can then be expressed with those too. E.g. a != b is the same as !(a==b). The same can be used for the arithmetic operators.

Constness

In addition to declaring member methods const that don't change the objects state, also take parameter inputs as const&.

Surprising behaviour/Bugs

The constructors taking an (unsigned) int are different from the ones taking floating point values as both the nominator and denominator are set to the same value. You then reduce the fraction leading always to 1/1 no matter what input it takes. The denominator should be set to 1 instead.

The reduce method returns a copied fraction object implying that the original object is not changed. Otherwise the return type might as well be void or return a reference.

Why are you flushing the stream in your << operator? That really hurts performance if I wanted e.g. to print many fractions at the same time. Let the stream user decide when he wants to flush it.

Your * and / are wrong with respect to your is_neg member. If one operand is negative, the member is set to true. But if both operands are negative, the member should actually be false. Simple test case:

fraction f(-1.5);
fraction g(-1.5);
std::cout << f * g << '\n';
// prints -9/4 instead of 9/4
\$\endgroup\$
  • \$\begingroup\$ Greatly appreciate all the help. It means a lot. \$\endgroup\$ – G. Foster Jul 29 '17 at 21:21
4
\$\begingroup\$

Separate of Concerns

This is a fractions class.
It has nothing to do with user input. Remove all the references to std::cout, and don't ask the user questions if things go wrong. Throw an exception or some other appropriate action. Then in the code that uses your class detect these situations and fix in that.

If I have a server application that is using your code I would be really pissed off if you locked up the application waiting for user input (that will never come).

Arithmetic

Implementing operator+ and other operations.

This is usually done by implementing operator+= then defining operator+ in terms of that:

 class X
 {
     public:
         X& operator+=(X const& rhs)
         {
             // Action that implements +=
             // By adding the rhs into this side.
             return *this;
         }
         friend X operator+(X const& lhs, X const& rhs)
         {
             X  result(lhs);
             return result += rhs;    // Notice the use of +=
         }
};

Questions

So the default value is 1?

fraction::fraction()
{
    fraction_symbol = '/';
    is_reduced = false;
    is_neg = false;
    numerator = 1;
    denominator = 1;
}

In most other number classes, the default value is zero.

Getters/Setters break encapsulation

    int unsigned get_numerator();
    int unsigned get_denominator();
    void set_numerator(int);
    void set_denominator(int);

This is a class that holds numbers more exactly. How the implementation does that should be invisible to people outside the class. In a couple of years you could find a better technique for doing this.

But because you have these functions you are basically locking yourself to this implementation, thus destroying the whole reason for encapsulating the data.

Do you really need these methods? Why would I want to get them?

Non mutating functions should be const

If a method does not change the state of the object it should be marked const.

unsigned int fraction::get_numerator()  const // Const here
{
    return numerator;
}

unsigned int fraction::get_denominator() const // Const here
{
    return denominator;
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.