Number class to add/compare different kinds of numbers

I am creating a Number class having Integer and Fraction derive class which add/compare different kinds of numbers. In addition, the caller need not needs to know what kind of numbers are being added. For example, the solutions expected to be able to do the following:

• \$\frac{1}{2} + 2 = \frac{5}{2}\$ (fraction)

• \$\frac{2}{6} + \frac{2}{3} = 1\$ (integer)

The solution creates three classes: Number, Integer and Fraction and overloads the + and == operators.

Can someone please review my solution? I particularly wanted to know about the c++ design and how I can improve it. Also, is the use of unique_ptr correct?

Factory.h

#pragma once
#include "Number.h"
#include 
class Factory
{
public:

    std::unique_ptr createObj(int a);
    std::unique_ptr createObj(int a, int b);
    Factory() {}
    ~Factory() {}

};

Factory.cpp

#include "Factory.h"
#include "Integer.h"
#include "Fraction.h"
#include "Utility.h"

std::unique_ptr Factory::createObj(int a)
{
    if (utility::integeroverflow(a))
    {
        return make_unique(a);
    }
}

std::unique_ptr Factory::createObj(int a, int b)
{   
    if (utility::integeroverflow(a))
    {
        if (utility::dividebyzero(b))
        {
            return make_unique(a, b);
        }
    }
}

Number.h

#ifndef NUMBER_CLASS_H
#define NUMBER_CLASS_H

#include 
#include 
using namespace std;

class Integer;
class Fraction;

class Number
{

public:
    virtual void display(void) =0;

    virtual Integer * isInteger() { return NULL; }
    virtual Fraction * isFraction() { return NULL; }

    std::unique_ptr operator+ ( Number &);
    bool operator==( Number &);

    virtual ~Number() {};

};

#endif

Number.cpp

```
#include "Number.h"
#include "Integer.h"
#include "Fraction.h"

// overloading "=="
bool Number::operator==(Number &rhs)
{
Integer *intPtr1 = this->isInteger();
Integer *intPtr2 = rhs.isInteger();

Fraction *fracPtr1 = this->isFraction();
Fraction *fracPtr2 = rhs

I think you've got too much code there; certainly I didn't read all of it. But I'll take you on my journey through the parts I did read.

std::unique_ptr Factory::createObj(int a, int b)
{   
    if (utility::integeroverflow(a))
    {
        if (utility::dividebyzero(b))
        {
            return make_unique(a, b);
        }
    }
}

At first glance this seems nice and straightforward... it looks like you're using a classical-OOP class hierarchy where a Fraction is-a Number, which means you have to use heap allocation and unique_ptrs all over the place. Now, I personally don't like classical-OOP style in C++ (because it requires all that heap allocation), but sure, as a learning exercise, go for it. Your use of unique_ptr is indeed correct, as far as that goes.

However, I would expect a good programmer to write

if (a && b) {
    ...
}

instead of

if (a) {
    if (b) {
        ...
    }
}

But hang on, what is this if integeroverflow, if dividebyzero? That's actually quite unusual control flow you've got there! I infer that utility::dividebyzero is going to be some sort of guard against division by zero... but I can't figure out how you're going to make it work. Well, let's go look at it. (Actually, I looked at integeroverflow first.)

bool utility::integeroverflow(int a)
{
    if ((a > INT_MAX) || (a < INT_MIN)) // overflow
        throw std::overflow_error("Exception:Integer overflow");
    else
        return true;
}

Red flag. An int can never be greater than INT_MAX nor less than INT_MIN. And indeed, if you compile this with clang++ -O2, you get:

__ZN7utility15integeroverflowEi:
        pushq   %rbp
        movq    %rsp, %rbp
        movb    $1, %al
        popq    %rbp
        retq

The compiler is smart enough to know that this function is a no-op that always returns true.

So, you've got some fundamental misconceptions about what an int is in C++. On your platform, it's a 32-bit quantity, and it has no "not-a-number" values — it's not like floating-point. You should go read up on bits and bytes, and maybe start your math programming with something a lot simpler, like the old classic "Fahrenheit to Celsius temperature converter".

Furthermore, once I saw what you were trying to do, it was apparent that utility::dividebyzero(b) wouldn't be good enough either. Try the expression Factory::createObj(1, -1) + Factory::createObj(1, INT_MIN)... can you figure out why it crashes?

Furthermore, once I actually started writing up this answer, and had to cut-and-paste the code above, I noticed something that your compiler already warned you about — you should go fix all the compiler warnings immediately! Don't think "oh, I know what I'm doing, the compiler is wrong." At your level of experience, the compiler is always right, with a margin of error of plus or minus 1 in a billion.

The function Factory::createObj(int a, int b) doesn't return a value along all of its possible codepaths. You can fix this by adding return std::make_unique(1,1); at the bottom, or by adding __builtin_unreachable(); at the bottom, or by refactoring your dividebyzero etc functions to be assertions rather than conditions, i.e.

std::unique_ptr Factory::createObj(int a, int b)
{
    utility::throw_if_overflow(a);  // see above as to why this is silly
    utility::throw_if_zero(b);
    return std::make_unique(a, b);
}

Not only will the compiler be happy with this, but it'll generate more efficient code (in unoptimized builds), and it's several lines shorter as well, which means you can fit more code on your screen, which means you can keep more of your program in view at one time. (I also picked you some clearer function names while I was at it.)

Anyway, that's all I bothered to look at, since I assume most of the mass of code you posted is occupied with nonsense similar to utility::integeroverflow(int).

BTW, if you can find a used copy of The Elements of Programming Style, I highly recommend picking it up and reading it cover to cover. You'll learn a lot about programming, and the bonus is that it'll be harsh critiques of other people's code instead of yours. ;)

StackExchange Code Review Q#139008, answer score: 3