#ifndef FRACTION_H #define FRACTION_H 

Your guard name is vulnerable to collisions. Append more information and a good differentiator to reduce the chance a collision occurs.

// PROJECT_FILENAME_CREATIONDATE_CREATIONAUTHOR (GUID, etc) #ifndef MYPROJ_FRACTION_H_042018_ANNAPHUONG 

---

Fraction::Fraction() :m_Numerator(0), m_Denominator(0) {} 

Did you intend for 0 to be in the denominator? Consider the following code:

Fraction f; std::cout << f.toDouble() << '\n'; // prints -nan 

If your default constructed Fraction is supposed to be equal to zero, you should initialize the denominator to 1.

If you are going to initialize your members to constants, prefer the in-class initializers.

class Fraction { Fraction() = default; // required since you provide Fraction(int n, int d); // <-- another constructor private: int m_Numerator {0}; // 0 if other ctors don't initialize int m_Denominator {1}; // 1 if other ctors don't initialize }; 

---

Fraction::Fraction(int numerator, int denominator) :m_Numerator(numerator), m_Denominator(denominator) { } 

If you don't intend to define the behavior for division by zero, you may want to check the 2nd argument.

Have you considered providing a single argument constructor for a whole number? i.e. Fraction f(42);.

---

void set(int numberator, int denominator); 

Since you are working with int as your underlying type, there is no reason to allow the user to reuse an existing Fraction. If you were using a type that allowed for infinite precision (modulo available memory), perhaps it would then be appropriate. You take an immutable approach with every other function, so I would remain consistent and remove this setter.

---

int Fraction::findGCD(int a, int b) { if (a == 0) return b; return findGCD(b%a, a); } 

findGCD doesn't require knowledge of the internal representation of your fraction. I would make this a free-standing non-member function.

---

double Fraction::toDouble() const { return ((double) m_Numerator/m_Denominator); } QString Fraction::toString()const { return QString("%1 / %2 = %3") .arg(m_Numerator) .arg(m_Denominator) .arg(toDouble()); } 

You could allow users to implement any additional required functionality if you provided num() and den() accessors.

template <typename Float> Float fraction_cast(Fraction const f) { return static_cast<Float>(f.num()) / f.den(); } template <> QString fraction_cast<QString>(Fraction const f) { return QString("%1 / %2").arg(f.num()).arg(f.den()); } int main() { Fraction f1(3, 2); std::cout << fraction_cast<QString>(f1) << '\n'; // prints "3 / 2" std::cout << fraction_cast<double>(f1) << '\n'; // prints "1.5" } 

Your Fraction is no longer dependent on QString, only the QString specialization for fraction_cast<> is.

---

Fraction Fraction::simplify() { int gcd = findGCD(m_Numerator, m_Denominator); return Fraction::Fraction(m_Numerator / gcd, m_Denominator / gcd); } 

Doesn't make any changes to the Fraction itself, consider const-correctness. If the denominator ever became negative, do you plan on simplifying (normalizing) the sign to the numerator?

---

 Fraction add(const Fraction& other); Fraction substract(const Fraction& other); Fraction multiphy(const Fraction& other); Fraction divide(const Fraction& other); 

I would suggest overloading the arithmetic operators for each of these.

 const Fraction operator+(const Fraction&) const; const Fraction operator-(const Fraction&) const; const Fraction operator*(const Fraction&) const; const Fraction operator/(const Fraction&) const; 

Also, some non-member free-standing functions, like comparisons, math functions (abs, sqrt, pow, invert, truncate, modulus, negate), boolean conversion (including operator!), increment/decrement, sign determination, etc.