From c95660961ddb256827d348a7db20c8a3de79e100 Mon Sep 17 00:00:00 2001 From: Alex Vergara Date: Tue, 10 Sep 2024 18:08:43 +0200 Subject: [PATCH] feat: arithmetic division --- zero/ifc/math/general.cppm | 3 +- zero/ifc/math/numbers/detail.cpp | 98 +++++++++++++--------------- zero/ifc/math/numbers/naturals.cppm | 19 ++---- zero/ifc/math/numbers/numbers.cppm | 54 ++++++++------- zero/ifc/math/numbers/rationals.cppm | 23 ++++--- zero/tests/math/numbers_tests.cpp | 29 +++++--- zork_config/zork_clang.toml | 4 +- 7 files changed, 119 insertions(+), 111 deletions(-) diff --git a/zero/ifc/math/general.cppm b/zero/ifc/math/general.cppm index ec4e9ba..3e989c1 100644 --- a/zero/ifc/math/general.cppm +++ b/zero/ifc/math/general.cppm @@ -42,5 +42,6 @@ export namespace zero::math { { T::symbol } -> std::same_as; // Check if 'T::symbol' has the type MathSymbol } ); - template concept Numerical = Number || std::is_arithmetic_v; + template + concept Numerical = Number> || std::is_arithmetic_v>; } diff --git a/zero/ifc/math/numbers/detail.cpp b/zero/ifc/math/numbers/detail.cpp index 13ac750..c316aa2 100644 --- a/zero/ifc/math/numbers/detail.cpp +++ b/zero/ifc/math/numbers/detail.cpp @@ -43,10 +43,37 @@ constexpr auto arithmetic_op(const L &lhs, const R &rhs, Op op) noexcept { return op(normalize(lhs), normalize(rhs)); } +/** @brief Helper function to sum or subtract two Rationals + * @details it should be placed **before** the `rational_add` and rational_subtract` functions for being + * compilable with MSVC, as it has stricter name lookup resolution rules + */ +[[nodiscard]] constexpr Rational +sum_or_subtract(const Rational &lhs, const Rational &rhs, + const ArithmeticOperation op) noexcept { + const int sign = op == ArithmeticOperation::Add ? 1 : -1; + + const int lhs_numerator = lhs.numerator().number(); + const int rhs_numerator = sign * rhs.numerator().number(); + const int lhs_denominator = lhs.denominator().number(); + const int rhs_denominator = rhs.denominator().number(); + + if (lhs_denominator == rhs_denominator) { // Like fractions + return {lhs_numerator + rhs_numerator, lhs_denominator}; + } else { // Unlike fractions + // Get their LCD by finding their LCM + const auto lcd = zero::math::lcm(lhs_denominator, rhs_denominator); + + // Scale numerators to have the common denominator (LCM) + const int numerator = (lhs_numerator * (lcd / lhs_denominator)) + + (rhs_numerator * (lcd / rhs_denominator)); + + return {numerator, lcd}; + } +} + // Specialized addition and subtraction for Rational types -template -constexpr auto rational_add(const L &lhs, const R &rhs) noexcept { - const auto op = ArithmeticOperation::Add; +template +constexpr auto rational_add_or_subtract(const L &lhs, const R &rhs, Op op) noexcept { if constexpr (std::is_same_v && std::is_same_v) return sum_or_subtract(lhs, rhs, op); else if constexpr (std::is_same_v) @@ -56,64 +83,31 @@ constexpr auto rational_add(const L &lhs, const R &rhs) noexcept { } template -constexpr auto rational_subtract(const L &lhs, const R &rhs) noexcept { - const auto op = ArithmeticOperation::Subtract; - if constexpr (std::is_same_v && std::is_same_v) - return sum_or_subtract(lhs, rhs, op); - else if constexpr (std::is_same_v) - return sum_or_subtract(lhs, Rational(rhs), op); - else if constexpr (std::is_same_v) - return sum_or_subtract(Rational(lhs), rhs, op); +constexpr auto rational_multiplication(const L &lhs, const R &rhs) noexcept { + const Rational _lhs = Rational(lhs); + const Rational _rhs = Rational(rhs); + + return Rational(_lhs.numerator().number() * _rhs.numerator().number(), + _lhs.denominator().number() * _rhs.denominator().number()); } template -constexpr auto rational_mult(const L &lhs, const R &rhs) noexcept { - if constexpr (std::is_same_v, Rational> && - std::is_same_v, Rational>) - return Rational(lhs.numerator() * rhs.numerator(), - lhs.denominator() * rhs.denominator()); - else if constexpr (std::is_same_v, Rational>) - return Rational(lhs.numerator() * normalize(rhs), lhs.denominator()); - else if constexpr (std::is_same_v, Rational>) - return Rational(normalize(lhs) * rhs.numerator(), rhs.denominator()); +constexpr auto rational_division(const L &lhs, const R &rhs) noexcept { + const Rational _lhs = Rational(lhs); + const Rational _rhs = Rational(rhs); + + return Rational(_lhs.numerator().number() * _rhs.denominator().number(), + _lhs.denominator().number() * _rhs.numerator().number()); } // Equality check for Rational types template constexpr auto rational_equality(const L &lhs, const R &rhs) noexcept { - if constexpr (std::is_same_v, Rational> && - std::is_same_v, Rational>) - return lhs.numerator() == rhs.numerator() && - lhs.denominator() == rhs.denominator(); - else if constexpr (std::is_same_v, Rational>) - return lhs == Rational(rhs); - else if constexpr (std::is_same_v, Rational>) - return Rational(lhs) == rhs; -} - -// Helper function to sum or subtract two Rationals -[[nodiscard]] constexpr Rational -sum_or_subtract(const Rational &lhs, const Rational &rhs, - const ArithmeticOperation op) noexcept { - const int sign = op == ArithmeticOperation::Add ? 1 : -1; - - const int lhs_numerator = lhs.numerator().number(); - const int rhs_numerator = sign * rhs.numerator().number(); - const int lhs_denominator = lhs.denominator().number(); - const int rhs_denominator = rhs.denominator().number(); - - if (lhs_denominator == rhs_denominator) { // Like fractions - return {lhs_numerator + rhs_numerator, lhs_denominator}; - } else { // Unlike fractions - // Get their LCD by finding their LCM - const auto lcd = zero::math::lcm(lhs_denominator, rhs_denominator); - - // Scale numerators to have the common denominator (LCM) - const int numerator = (lhs_numerator * (lcd / lhs_denominator)) + - (rhs_numerator * (lcd / rhs_denominator)); + const Rational _lhs = Rational(lhs); + const Rational _rhs = Rational(rhs); - return {numerator, lcd}; - } + return _lhs.numerator().number() == _rhs.numerator().number() && + _lhs.denominator().number() == _rhs.denominator().number(); } #if defined(__clang__) diff --git a/zero/ifc/math/numbers/naturals.cppm b/zero/ifc/math/numbers/naturals.cppm index 3b6eabc..5e46fd2 100644 --- a/zero/ifc/math/numbers/naturals.cppm +++ b/zero/ifc/math/numbers/naturals.cppm @@ -11,31 +11,20 @@ export namespace zero::math { private: // TODO: shouldn't be unsigned. we may decide what kind of thing we do // with signedness - unsigned int _number; + int _number; public: constexpr static MathSymbol symbol { MathSymbol::Naturals }; - [[nodiscard]] constexpr explicit Natural(unsigned int value) noexcept : _number(value) {} + [[nodiscard]] constexpr explicit Natural(const int value) noexcept : _number(value) {} /// @returns an {@link unsigned int}, which is the value stored in the type, being only a positive integer number - [[nodiscard]] constexpr unsigned int number() const noexcept { return _number; } + [[nodiscard]] constexpr int number() const noexcept { return _number; } /// TODO: should we do something about the values < 1? - /// Definetly yes, and now that we have a common base via CRTP, - /// we can override the impl on naturals to provide custom behaviour - /// + /// Maybe promote them to Integer or just throw? /* [[nodiscard]] Natural Natural::operator-(const Natural rhs) const noexcept { return Natural(_number - rhs.number()); } */ - /// @overload - /* [[nodiscard]] bool operator==(unsigned int rhs) const noexcept { - return _number == rhs; // TODO: I think that we don't need this one - } */ - // Printable // TODO: please, add a concept for this operators - friend std::ostream& operator<<(std::ostream& os, const Natural& rhs) { - os << rhs._number; - return os; - } }; } diff --git a/zero/ifc/math/numbers/numbers.cppm b/zero/ifc/math/numbers/numbers.cppm index 4fb9695..2742b84 100644 --- a/zero/ifc/math/numbers/numbers.cppm +++ b/zero/ifc/math/numbers/numbers.cppm @@ -14,16 +14,20 @@ export import :general; import :numbers.detail; - export namespace zero::math { + +// TODO: on the rational operations between Rational and a non-rational, +// we can definitely make the impl simpler by promoting the non rational +// to Rational (standalone templated helper that casts both to rationals +// maybe without if constexpr branches) + template constexpr auto operator+(const L &lhs, const R &rhs) noexcept { auto op = [](const auto &a, const auto &b) { - if constexpr (EitherRational) { - return rational_add(a, b); - } else { + if constexpr (EitherRational) + return rational_add_or_subtract(a, b, ArithmeticOperation::Add); + else return a + b; - } }; return arithmetic_op(lhs, rhs, op); } @@ -31,11 +35,10 @@ constexpr auto operator+(const L &lhs, const R &rhs) noexcept { template constexpr auto operator-(const L &lhs, const R &rhs) noexcept { auto op = [](const auto &a, const auto &b) { - if constexpr (EitherRational) { - return rational_subtract(a, b); - } else { + if constexpr (EitherRational) + return rational_add_or_subtract(a, b, ArithmeticOperation::Subtract); + else return a - b; - } }; return arithmetic_op(lhs, rhs, op); } @@ -43,33 +46,38 @@ constexpr auto operator-(const L &lhs, const R &rhs) noexcept { template constexpr auto operator*(const L &lhs, const R &rhs) noexcept { auto op = [](const auto &a, const auto &b) { - if constexpr (EitherRational) { - return rational_mult(a, b); - } else { + if constexpr (EitherRational) + return rational_multiplication(a, b); + else return a * b; - } }; return arithmetic_op(lhs, rhs, op); } - -// constexpr auto operator/(const L &lhs, const R &rhs) { +template +constexpr auto operator/(const L &lhs, const R &rhs) noexcept { + auto op = [](const auto &a, const auto &b) { + if constexpr (EitherRational) + return rational_division(a, b); + else + return a / b; + }; + return arithmetic_op(lhs, rhs, op); +} template constexpr bool operator==(const L &lhs, const R &rhs) noexcept { auto op = [](const auto &a, const auto &b) { - if constexpr (EitherRational) { - } else { + if constexpr (EitherRational) + return rational_equality(a, b); + else return a == b; - } }; return arithmetic_op(lhs, rhs, op); } - -template -std::ostream &operator<<(std::ostream& os, const N& n) { - os << n.number(); - return os; +template std::ostream &operator<<(std::ostream &os, const N &n) { + os << n.number(); + return os; } } // namespace zero::math diff --git a/zero/ifc/math/numbers/rationals.cppm b/zero/ifc/math/numbers/rationals.cppm index da501ce..63d574e 100644 --- a/zero/ifc/math/numbers/rationals.cppm +++ b/zero/ifc/math/numbers/rationals.cppm @@ -34,14 +34,19 @@ export namespace zero::math { public: constexpr static MathSymbol symbol = MathSymbol::Rationals; - /* template // TODO: don't think that's work to have universal references over const l-value references + template // TODO: don't think that's work to have universal references over const l-value references [[nodiscard]] constexpr Rational(L&& numerator, R&& denominator) noexcept : _numerator(static_cast(std::forward(numerator))), _denominator(static_cast(std::forward(denominator))) {} -*/ - template + + template // TODO: don't think that's work to have universal references over const l-value references + [[nodiscard]] constexpr Rational(N&& numerator) noexcept + : _numerator(static_cast(std::forward(numerator))), + _denominator(1) {} + + /* template [[nodiscard]] constexpr Rational(const L& numerator, const R& denominator) noexcept - : _numerator(static_cast(numerator)), _denominator(static_cast(denominator)) {} + : _numerator(static_cast(numerator)), _denominator(static_cast(denominator)) {} */ [[nodiscard]] constexpr Rational(const Rational& other) noexcept = default; [[nodiscard]] constexpr Rational(Rational&& other) noexcept = default; @@ -55,12 +60,12 @@ export namespace zero::math { // TODO: Add a method to reduce fractions // Comparison operator overloads - // TODO should we check that 4/2 is the same as 2/1 right? Or we should maintain the difference and explicitly + // TODO: should we check that 4/2 is the same as 2/1 right? Or we should maintain the difference and explicitly // say that 4/2 aren't the same Rational number as 2/1? - [[nodiscard]] bool operator==(const Rational rhs) const noexcept { - // return _numerator == rhs.numerator() && _denominator == rhs.denominator(); - return _numerator.number() == rhs.numerator().number() && _denominator.number() == rhs.denominator().number(); - } +// [[nodiscard]] bool operator==(const Rational rhs) const noexcept { +// // return _numerator == rhs.numerator() && _denominator == rhs.denominator(); +// return _numerator.number() == rhs.numerator().number() && _denominator.number() == rhs.denominator().number(); +// } // Printable friend std::ostream &operator<<(std::ostream& os, const Rational& rhs) { diff --git a/zero/tests/math/numbers_tests.cpp b/zero/tests/math/numbers_tests.cpp index dd6c5a4..6af060a 100644 --- a/zero/tests/math/numbers_tests.cpp +++ b/zero/tests/math/numbers_tests.cpp @@ -14,7 +14,7 @@ static_assert(!Number); void numbers_tests() { TEST_CASE(numbers_suite, "Testing the Numbers types construction", [] { auto natural = Natural(1); - assertEquals(1u, natural.number()); + assertEquals(1, natural.number()); auto integer = Integer(7); assertEquals(7, integer); @@ -40,6 +40,8 @@ void numbers_tests() { assertEquals(Integer(-1), Integer(-1)); assertEquals(Rational(1, 2), Rational(1, 2)); + // Equality check for equal ratio values returns false if they aren't the exact + // same rational number assertNotEquals(Rational(1, 2), Rational(2, 4)); }); @@ -47,20 +49,23 @@ void numbers_tests() { auto one_natural = Natural(5); auto other_natural = Natural(2); - assertEquals(7u, one_natural + other_natural); - assertEquals(3u, one_natural - other_natural); - assertEquals(10u, one_natural * other_natural); - // TODO division + assertEquals(7, one_natural + other_natural); + assertEquals(3, one_natural - other_natural); + assertEquals(10, one_natural * other_natural); + assertEquals(2, one_natural / other_natural); // int division. By default, this operator is the same as the + // language defined, so it will truncate the integer division + // towards zero. This is the same for any non-rational Number type }); TEST_CASE(numbers_suite, "Arithmetic operations with Integers", [] { - auto one_integer = Integer(10); - auto other_integer = Integer(20); + auto one_integer = Integer(20); + auto other_integer = Integer(10); assertEquals(30, one_integer + other_integer); - assertEquals(-10, one_integer - other_integer); + assertEquals(10, one_integer - other_integer); + assertEquals(-10, other_integer - one_integer); assertEquals(200, one_integer * other_integer); - // TODO division + assertEquals(2, one_integer / other_integer); }); TEST_CASE(numbers_suite, "Arithmetic operations with Rationals (like fractions)", [] { @@ -75,6 +80,9 @@ void numbers_tests() { auto rational_multiplication = one_rational * other_rational; assertEquals(Rational(32, 4), rational_multiplication); + + auto rational_division = one_rational / other_rational; + assertEquals(Rational(16, 8), rational_division); }); TEST_CASE(numbers_suite, "Arithmetic operations with Rationals (unlike fractions)", [] { @@ -95,5 +103,8 @@ void numbers_tests() { auto rational_times_integer = one_rational * Integer(7); assertEquals(Rational(21, 2), rational_times_integer); + + auto rational_divided_by_integer = one_rational / Integer(7); + assertEquals(Rational(3, 14), rational_divided_by_integer); }); } diff --git a/zork_config/zork_clang.toml b/zork_config/zork_clang.toml index 4cce6d9..3f161a2 100644 --- a/zork_config/zork_clang.toml +++ b/zork_config/zork_clang.toml @@ -9,8 +9,8 @@ cpp_compiler = "clang" driver_path = "clang++" # This binary is soft linked and included in our local path cpp_standard = "23" std_lib = "LIBCPP" -std_lib_installed_dir = "/usr/local/share/libc++/v1" -# std_lib_installed_dir = "C:/msys64/clang64/share/libc++/v1" +# std_lib_installed_dir = "/usr/local/share/libc++/v1" +std_lib_installed_dir = "C:/msys64/clang64/share/libc++/v1" extra_args = [ '-Werror', '-Wall', '-Wpedantic', '-pedantic', '-Wextra', '-Wconversion', '-Wfloat-conversion', '-Wsign-conversion', '-Wshadow', '-Wnon-virtual-dtor', '-Wold-style-cast', '-Wcast-align', '-Wunused', '-Woverloaded-virtual',