RattleInGlasses / ps_oop

0 stars 0 forks source link

Замечания по задаче с рациональными числами #15

Open alexey-malov opened 9 years ago

alexey-malov commented 9 years ago

Рекурсию в поиске НОД заменить на итерацию. Тесты должны помочь убедиться в том, что все нормально.

alexey-malov commented 9 years ago

Создание классов, состоящих только из статических функций, несет относительно мало пользы по сравнению со свободными функциям (возможно в собственном пространстве имен).

alexey-malov commented 9 years ago
    CRational();
    CRational(int value);
    CRational(int numerator, int denominator);

можно заменить на 1, с дефолтными параметрами

CRational(int numerator = 0, int denominator = 1);
alexey-malov commented 9 years ago
std::pair<int, CRational> CRational::ToCompoundFraction() const
{
    std::pair<int, CRational> result;

    result.first = m_numerator / m_denominator;
    result.second = *this - result.first;

    return result;
}

можно внутри этого метода использовать обычные переменные для хранения целой и дробной части, а результат вернуть либо через make_pair, либо через initializer_list:

std::pair<int, CRational> CRational::ToCompoundFraction() const
{
    auto integerPart = m_numerator / m_denominator;
    auto fractionalPart = *this - integerPart;

    return {integerPart, fractionalPart};
}
alexey-malov commented 9 years ago

В .cpp файле можно написать using namespace std, чтобы не указыать его явно

alexey-malov commented 9 years ago

Чтобы не быть многословным лучше объявить

class CRational
{
...
 typedef  std::pair<int, CRational> CompoundFraction;
...
};
typedef CRational::CompoundFraction CompoundFraction;

это улучшит также читаемость кода

alexey-malov commented 9 years ago
friend void ToCommonDenominator(CRational &num1, CRational &num2);

Дружественную функцию лучше заменить на статический метод