patterncppModerate
A minimal Fraction class in C++
Viewed 0 times
minimalclassfraction
Problem
I am trying to learn idiomatic Modern C++. I was wondering how you would improve upon this Fraction class. I would really appreciate any help with code organization, cleanliness and anything else that I should keep in mind.
Header:
Class file:
```
#include
#include "Fraction.h"
void Fraction::reduce() {
int gcd = calculate_gcd(numerator, denominator);
numerator = numerator / gcd;
denominator = denominator / gcd;
}
int Fraction::calculate_gcd(int a, int b) const {
int temp{};
a = std::abs(a);
b = std::abs(b);
if (b > a) {
temp = a;
a = b;
b = temp;
}
while(b != 0) {
temp = b;
b = a % b;
a = temp;
}
return a;
}
std::istream &operator>>(std::istream &is, Fraction &value) {
int numerator{}, denominator{};
is >> numerator >> denominator;
value.set_num(numerator);
value.set_den(denominator);
if (!is) {
numerator = 0;
denominator = 0;
}
return is;
}
std::ostream &operator (const Fraction &lhs, const Fraction &rhs) {
return (((lhs.get_num() rhs.get_den()) - (rhs.get_num() lhs.get_den())) > 0);
}
bool operator=(const Fraction &lhs, const Fraction &rhs
Header:
#ifndef FRACTION_H
#define FRACTION_H
#include
class Fraction {
int numerator{}, denominator{};
public:
Fraction() = default;
Fraction(int n, int d):
numerator(n), denominator(d) {}
~Fraction() = default;
void set_num(int value) { numerator = value; }
void set_den(int value) { denominator = value; }
int get_num() const { return numerator; }
int get_den() const { return denominator; }
void reduce();
int calculate_gcd(int, int) const;
//void display() {}
};
std::istream &operator>>(std::istream &is, Fraction &value);
std::ostream &operator (const Fraction &lhs, const Fraction &rhs);
bool operator=(const Fraction &lhs, const Fraction &rhs);
bool operator<=(const Fraction &lhs, const Fraction &rhs);
#endifClass file:
```
#include
#include "Fraction.h"
void Fraction::reduce() {
int gcd = calculate_gcd(numerator, denominator);
numerator = numerator / gcd;
denominator = denominator / gcd;
}
int Fraction::calculate_gcd(int a, int b) const {
int temp{};
a = std::abs(a);
b = std::abs(b);
if (b > a) {
temp = a;
a = b;
b = temp;
}
while(b != 0) {
temp = b;
b = a % b;
a = temp;
}
return a;
}
std::istream &operator>>(std::istream &is, Fraction &value) {
int numerator{}, denominator{};
is >> numerator >> denominator;
value.set_num(numerator);
value.set_den(denominator);
if (!is) {
numerator = 0;
denominator = 0;
}
return is;
}
std::ostream &operator (const Fraction &lhs, const Fraction &rhs) {
return (((lhs.get_num() rhs.get_den()) - (rhs.get_num() lhs.get_den())) > 0);
}
bool operator=(const Fraction &lhs, const Fraction &rhs
Solution
-
Unrestricted public getters and setters defeat the privacy of
-
The constructor should call
-
It is recommended to implement
-
It might be a matter of taste, but I advise against the precompiled headers. In the long run, they create more problems than they solve.
Unrestricted public getters and setters defeat the privacy of
numerator and denominator. I recommend dropping getters and setters altogether, and let the class methods access the data fields directly.-
The constructor should call
reduce. This way the fraction is always kept in the reduced form, and the operator== can be simplified tobool operator==(const Fraction &lhs, const Fraction &rhs) {
return (lhs.numerator == rhs.numerator) && (lhs.denominator == rhs.denominator);
}-
It is recommended to implement
operator> in terms of operator
-
There is no reason to publicly expose calculate_gcd and reduce`.-
It might be a matter of taste, but I advise against the precompiled headers. In the long run, they create more problems than they solve.
Code Snippets
bool operator==(const Fraction &lhs, const Fraction &rhs) {
return (lhs.numerator == rhs.numerator) && (lhs.denominator == rhs.denominator);
}bool operator> (const Fraction &lhs, const Fraction &rhs) {
return rhs < lhs;
}bool operator==(const Fraction &lhs, const Fraction &rhs) {
return !(lhs < hrs) && !(rhs < lhs);
}Context
StackExchange Code Review Q#162338, answer score: 17
Revisions (0)
No revisions yet.