I have one class Number
(base class) and two child
class Fraction
and Integer
. I am suppose to provide the functionality to add
and compare
two Integer obj, two Fraction obj, One Integer One Fraction obj by operator overloading. Below is what I have come up with. Can someone kindly review it. I am trying to avoid memory leaks too. Not sure if I succeeded.
1. Factory.h
class Factory
{
public:
Number * createObj(int a);
Number * createObj(int a, int b);
Factory(){}
};
2. Factory.cpp
Number * Factory::createObj(int a)
{
return new Integer(a);
}
Number * Factory::createObj(int a, int b)
{
return new Fraction(a, b);
}
3. Number.h
class Number
{
public:
virtual void display(std::ostream &) = 0;
virtual Integer * isInteger(){return 0;}
virtual Fraction * isFraction() { return 0; }
friend Number* operator+ (Number &, Number &);
friend bool operator==(Number &, Number &);
};
4. Number.cpp
// overloading "=="
bool operator==(Number &lhs, Number &rhs)
{
Integer *intPointer1;
Integer *intPointer2;
Fraction *fracPointer1;
Fraction *fracPointer2;
Display d1;
Number *sumNumberObj = NULL;
lhs.display(std::cout);
d1.printString(" and ");
rhs.display(std::cout);
// checking of two integers
if ((intPointer1 = lhs.isInteger()) && (intPointer2 = rhs.isInteger()))
{
if (*intPointer1 == *intPointer2)
return true;
return false;
}
// checking of two fraction
if (fracPointer1 = lhs.isFraction())
{
if (fracPointer2 = rhs.isFraction())
{
if (*fracPointer1 == *fracPointer2)
return true;
return false;
}
}
// checking of one integer and one fraction
if (intPointer1 = lhs.isInteger())
{
if (fracPointer1 = rhs.isFraction())
{
if (*intPointer1 == *fracPointer1)
return true;
return false;
}
}
// checking of fraction and integer
if (fracPointer1 = lhs.isFraction())
{
if (intPointer1 = rhs.isInteger())
{
if (*fracPointer1 == *intPointer1)
return true;
return false;
}
}
}
Number* operator+ (Number &lhs, Number &rhs)
{
Integer *intPointer1;
Integer *intPointer2;
Fraction *fracPointer1;
Fraction *fracPointer2;
Display d1;
Number *sumNumberObj = NULL;
// addition of two integers
if (intPointer1 = lhs.isInteger())
{
if (intPointer2 = rhs.isInteger())
{
sumNumberObj = *intPointer1 + *intPointer2;
d1.disp(lhs, rhs, *sumNumberObj);
}
}
// addition of two fraction
if (fracPointer1 = lhs.isFraction())
{
if (fracPointer2 = rhs.isFraction())
{
sumNumberObj = *fracPointer1 + *fracPointer2;
d1.disp(lhs, rhs, *sumNumberObj);
}
}
// addition of one integer and one fraction
if (intPointer1 = lhs.isInteger())
{
if (fracPointer1 = rhs.isFraction())
{
sumNumberObj = *intPointer1 + *fracPointer1;
d1.disp(lhs, rhs, *sumNumberObj);
}
}
// addition of fraction and integer
if (fracPointer1 = lhs.isFraction())
{
if (intPointer1 = rhs.isInteger())
{
sumNumberObj = *fracPointer1 + *intPointer1;
d1.disp(lhs, rhs, *sumNumberObj);
}
}
return sumNumberObj;
}
5. Fraction.h
class Fraction : public Number
{
Integer _numerator;
Integer _denominator;
public:
void display(std::ostream &);
Fraction() {}
Fraction(const int &, const int &);
Number* operator+ (const Integer&);
Number* operator+ (const Fraction&);
int gcdCalculate(int val1, int val2);
int lcmCalculate(const int val1, const int val2);
bool operator==(const Integer&) const;
bool operator==(const Fraction&)const;
Integer getnumerator() const;
Integer getdenominator()const;
virtual Fraction * isFraction() { return this; }
~Fraction();
};
6. Fraction.cpp
// parameterised constructor
Fraction::Fraction(const int & num, const int & den)
{
_numerator.setValue(num);
_denominator.setValue(den);
}
// destructor
Fraction::~Fraction()
{}
// display the value of number on output console
void Fraction::display(std::ostream & stream)
{
if (this->_denominator == 0)
std::cout << "Undefined: " << this->_numerator.getValue() << "/" << this->_denominator.getValue() << " (Divide By Zero Exception)";
else
stream << this->_numerator.getValue() << "/" << this->_denominator.getValue();
}
// "+" operator overloading for fraction and fraction
Number* Fraction::operator+ (const Fraction& numberTwo)
{
int lcm = lcmCalculate(this->_denominator.getValue(), numberTwo._denominator.getValue());
int multiplier1 = 0;
if (this->_denominator.getValue())
multiplier1 = lcm / this->_denominator.getValue();
int multiplier2 = 0;
if (numberTwo._denominator.getValue())
multiplier2 = lcm / numberTwo._denominator.getValue();
return new Fraction((this->_numerator.getValue() * multiplier1) + (numberTwo._numerator.getValue() * multiplier2), lcm);
}
// "+" operator overloading for fraction and integer
Number* Fraction::operator+(const Integer &secondNumber)
{
int num = this->getnumerator().getValue();
int den = this->getdenominator().getValue();
int numoriginal = secondNumber.getValue();
int numeratorVal = ((numoriginal * den) + num);
int denominatorVal = den;
return new Fraction(numeratorVal, denominatorVal);
}
// LCM Calculation
int Fraction::lcmCalculate(const int val1, const int val2)
{
int temp = gcdCalculate(val1, val2);
return temp ? (val1 / temp * val2) : 0;
}
// GCD Calculation
int Fraction::gcdCalculate(int val1, int val2)
{
for (;;)
{
if (val1 == 0) return val2;
val2 %= val1;
if (val2 == 0) return val1;
val1 %= val2;
}
}
// comparision operator overload for fraction and fraction
bool Fraction::operator==(const Fraction& rhs) const
{
Integer numCheck = this->_numerator;
Integer denCheck = this->_denominator;
if (rhs._numerator.getValue())
numCheck.setValue(numCheck.getValue() / rhs._numerator.getValue());
if (rhs._numerator.getValue())
denCheck.setValue(denCheck.getValue() / rhs._denominator.getValue());
if (numCheck == denCheck) {
return true;
}
return false;
}
// comparision operator overload for fraction and integer
bool Fraction::operator==(const Integer& rhs) const
{
int numFrac = this->getnumerator().getValue();
int denFrac = this->getdenominator().getValue();
int numVal = rhs.getValue();
if (denFrac == 0) {
std::cout << " Error: Undefined Value Undifined(0/0) ";
return false;
}
if (numVal == (numFrac / denFrac))
return true;
return false;
}
// getter for numberator
Integer Fraction::getnumerator() const
{
return this->_numerator;
}
// getter for denominator
Integer Fraction::getdenominator() const
{
return this->_denominator;
}
7. Integer.h
class Integer : public Number
{
int intValue;
public:
void display(std::ostream &);
int getValue() const;
void setValue(int);
Integer() {}
Integer(int num);
Number* operator+ (const Integer &);
Number* operator+ (const Fraction &);
~Integer();
bool operator==(const Fraction &) const;
bool operator==(const Integer &) const;
virtual Integer* isInteger() { return this; }
};
8. Integer.cpp
// parameterized constructor
Integer::Integer(int num)
{
intValue = num;
}
Integer::~Integer()
{}
// return integer value
int Integer::getValue() const
{
return this->intValue;
}
void Integer::setValue(int x)
{
this->intValue = x;
}
// operator "+" overloading
Number* Integer::operator+(const Integer &secondNumber)
{
int temp = this->intValue + secondNumber.intValue;
return new Integer(temp);
}
Number* Integer::operator+( const Fraction& secondNumber)
{
int num = secondNumber.getnumerator().getValue();
int den = secondNumber.getdenominator().getValue();
int numoriginal = this->getValue();
int numeratorVal = ((numoriginal * den) + num);
int denominatorVal = den;
return new Fraction(numeratorVal, denominatorVal);
}
// operator "=" overloading
void Integer::display(std::ostream& stream)
{
stream << this->intValue;
}
// comparasion operator overload
bool Integer::operator==(const Fraction& rhs) const
{
int numFrac = rhs.getnumerator().getValue();
int denFrac = rhs.getdenominator().getValue();
int numVal = this->getValue();
if (denFrac == 0) {
std::cout << " Error: Undefined Value Undifined(0/0) ";
return false;
}
if (numVal == (numFrac / denFrac))
return true;
return false;
}
bool Integer::operator==(const Integer& rhs) const
{
return this->intValue == rhs.getValue();
}
9. Display.h
class Display
{
public:
Display() {}
void printHeader(const std::string header) const;
void printString(const std::string str) const;
void calculateSum(Number&, Number &);
void disp(Number &, Number &, Number &) const;
void checkEqual(Number &, Number &) const;
~Display() {}
};
10. Display.cpp
// printing header banner to console
void Display::printHeader(const std::string header) const
{
std::cout << std::endl << std::endl << std::string(55, '*') << std::endl <<
header << std::endl << std::string(55, '*') << std::endl;
}
// prints the string passed to it
void Display::printString(const std::string str) const
{
std::cout << str;
}
// calling matching "+" overload as per the arguments
void Display::calculateSum(Number &num1, Number &num2)
{
std::auto_ptr<Number> result (num1 + num2);
}
// displaying appropriate result of returned bool value from "==" overload
void Display::checkEqual(Number &lhs, Number &rhs) const
{
Display d1;
if (lhs == rhs)
d1.printString(" : Numbers are Equal\n");
else
d1.printString(" : Numbers are Unequal\n");
}
// displaying the summation on console
void Display::disp(Number &lhs, Number &rhs, Number &sumNumberObj) const
{
Display d1;
lhs.display(std::cout);
d1.printString(" + ");
rhs.display(std::cout);
d1.printString(" = ");
sumNumberObj.display(std::cout);
d1.printString("\n");
}
11. Main.cpp
int main()
{
try
{
Display disp;
Factory *facObj = new Factory();
disp.printHeader("TESTING ADDTION OF TWO INTEGERS");
std::auto_ptr<Number> numberObjPointer1(facObj->createObj(5));
std::auto_ptr<Number> numberObjPointer2(facObj->createObj(7));
disp.calculateSum(*numberObjPointer1, *numberObjPointer2);
disp.printHeader("TESTING ADDTION OF TWO FRACTIONS");
std::auto_ptr<Number> numberObjPointer3(facObj->createObj(1, 5));
std::auto_ptr<Number> numberObjPointer4(facObj->createObj(1, 7));
disp.calculateSum(*numberObjPointer3, *numberObjPointer4);
disp.printHeader("TESTING ADDTION OF INTEGER AND FRACTIONS");
std::auto_ptr<Number> numberObjPointer5(facObj->createObj(2));
std::auto_ptr<Number> numberObjPointer6(facObj->createObj(3, 2));
disp.calculateSum(*numberObjPointer15, *numberObjPointer6);
disp.printHeader("TESTING ADDTION OF FRACTION AND INTEGER");
std::auto_ptr<Number> numberObjPointer7 (facObj->createObj(2,5));
std::auto_ptr<Number> numberObjPointer8 (facObj->createObj(3));
disp.calculateSum(*numberObjPointer7, *numberObjPointer8);
disp.printHeader("TESTING EQUALITY OF INTEGER AND INTEGER");
std::auto_ptr<Number> numberObjPointer9(facObj->createObj(2));
std::auto_ptr<Number> numberObjPointer10(facObj->createObj(2));
disp.checkEqual(*numberObjPointer9, *numberObjPointer10);
disp.printHeader("TESTING EQUALITY OF FRACTIONS");
std::auto_ptr<Number> numberObjPointer11(facObj->createObj(1,2));
std::auto_ptr<Number> numberObjPointer12(facObj->createObj(2,2));
disp.checkEqual(*numberObjPointer11, *numberObjPointer12);
disp.printHeader("TESTING INTEGER AND FRACTION ARE EQUAL");
std::auto_ptr <Number> numberObjPointer13 (facObj->createObj(2));
std::auto_ptr<Number> numberObjPointer14 (facObj->createObj(3, 2));
disp.checkEqual(*numberObjPointer13, *numberObjPointer14);
disp.printHeader("TESTING FRACTION AND INTEGER ARE EQUAL");
std::auto_ptr<Number> numberObjPointer15 (facObj->createObj(1));
std::auto_ptr<Number> numberObjPointer16 (facObj->createObj(8, 2));
disp.checkEqual(*numberObjPointer15, *numberObjPointer16);
}
catch (std::exception& ex)
{
std::cout << "\n " << ex.what() << std::endl;
}
return 0;
}
1 Answer 1
Code is way too complicated, but let me try:
Style
you should NOT use names starting with underscore. I did same error in the near past.
If you want to emphasize that those are private
,
add the underscore at the end.
Mixed case file names
You should avoid capital letters in file names. Windows
and MacOS
support case insensitive file names, but Linux
and most UNIX
-es - do not.
Imagine someone compress the code using ZIP
or ARJ
and when you un-compress it - surprise - capital/small letters are lost.
Once I lost all small letters on huge Java project - file names became all-caps - it was easy fixable since all I needed to do is to rename everything with small letters.
Keep class names with capital letter, but make files with small letters. C++
allow this, Java
do not.
This is huge problem in Java
world, but nobody speaking about it.
Factory.h
no need to provide default c-tor. it can be removed.
Number.h
I don't think this compiles.
virtual Integer * isInteger(){return 0;}
virtual Fraction * isFraction() { return 0; }
You need to use class Integer;
prior class Number
declaration.
Because you are playing with dynamically allocate objects + polymorphism, you MUST add virtual d-tor
. I do not think the code will work correctly without it.
virtual ~Number(){}
I do not like these two:
virtual Integer * isInteger(){return 0;}
virtual Fraction * isFraction() { return 0; }
Is this conversion or just check if class is Integer / Number?
Naming is unfortunate. Change it it toInteger()
or change return type to bool
.
What is return 0
? If it is a pointer, change it to return NULL;
or if you are using C++11
to return nullptr;
.
Number.cpp
way too big, skipping for the moment.
Fraction.h
once again this will not be compilled, because you are using unknown class Integer
.
default c-tor
must initialize the _numerator
and _denominator
. I suggest following:
Fraction() : _numerator(0), _denominator(1)
{}
or much better:
class Fraction : public Number
{
Integer _numerator = 0;
Integer _denominator = 1;
//...
Fraction() = default;
What is the d-tor
doing? Is this because of incomplete class Integer
.
C++11
comment - remove virtual
, use override
or final
.
final
helps the optimizer a lot to speed up the code, when class is known.
division by zero
- I think you need to check somehow for it. Probably in c-tor
.
Fraction.cpp
prefer initialize list:
Fraction::Fraction(const int & num, const int & den)
{
_numerator.setValue(num);
_denominator.setValue(den);
}
must be:
Fraction::Fraction(const int & num, const int & den) :
_numerator(num),
_denominator(den)
{
}
I would even move it to the Fraction.h
file.
skipping for now
Integer.h
same considerations for the c-tor
and d-tor
. Initialize the value to zero.
If you do so, you can ommit initialization of Fraction::_numerator
.
d-tor
once again doing nothing. this time you can remove it.
Display.h
remove c-tor
and d-tor
.
on all your code, you use pointers
. now suddenly you switch to refferences
.
Why? Answer is - to make main()
more ugly :)
main()
Do you think there will be exception thrown?
std::auto_ptr
is deprecated. If you are using C++11
, change it to std::unique_ptr
.
Make std::auto_ptr<Number>
to be typedef
. You really not need to write it several times.
Explore related questions
See similar questions with these tags.
Number.h
compiles? I do not think so \$\endgroup\$