From d28c25c7df571f885347e6558387fe19a95348a3 Mon Sep 17 00:00:00 2001
From: "piotr.idzik" <vil02@o2.pl>
Date: Fri, 19 May 2023 17:20:59 +0200
Subject: [PATCH 01/21] fix: set proper size of fac

---
 math/ncr_modulo_p.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/math/ncr_modulo_p.cpp b/math/ncr_modulo_p.cpp
index 03aecf67e88..008227425cb 100644
--- a/math/ncr_modulo_p.cpp
+++ b/math/ncr_modulo_p.cpp
@@ -40,7 +40,7 @@ class NCRModuloP {
      */
     NCRModuloP(const uint64_t& size, const uint64_t& mod) {
         p = mod;
-        fac = std::vector<uint64_t>(size);
+        fac = std::vector<uint64_t>(size + 1);
         fac[0] = 1;
         for (int i = 1; i <= size; i++) {
             fac[i] = (fac[i - 1] * i) % p;

From 5864c5af4c8653a8ddaa344b49f5167212a61189 Mon Sep 17 00:00:00 2001
From: "piotr.idzik" <vil02@o2.pl>
Date: Fri, 19 May 2023 17:23:26 +0200
Subject: [PATCH 02/21] style: use std::size_t as a type of loop counter

---
 math/ncr_modulo_p.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/math/ncr_modulo_p.cpp b/math/ncr_modulo_p.cpp
index 008227425cb..3bf070d3bc4 100644
--- a/math/ncr_modulo_p.cpp
+++ b/math/ncr_modulo_p.cpp
@@ -42,7 +42,7 @@ class NCRModuloP {
         p = mod;
         fac = std::vector<uint64_t>(size + 1);
         fac[0] = 1;
-        for (int i = 1; i <= size; i++) {
+        for (std::size_t i = 1; i <= size; i++) {
             fac[i] = (fac[i - 1] * i) % p;
         }
     }

From 81b3e7b95a810c5345024cfe2c2076405263a106 Mon Sep 17 00:00:00 2001
From: "piotr.idzik" <vil02@o2.pl>
Date: Fri, 19 May 2023 17:23:59 +0200
Subject: [PATCH 03/21] style: use uint64_t as a type of loop counter

---
 math/ncr_modulo_p.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/math/ncr_modulo_p.cpp b/math/ncr_modulo_p.cpp
index 3bf070d3bc4..b7bc1b30835 100644
--- a/math/ncr_modulo_p.cpp
+++ b/math/ncr_modulo_p.cpp
@@ -42,7 +42,7 @@ class NCRModuloP {
         p = mod;
         fac = std::vector<uint64_t>(size + 1);
         fac[0] = 1;
-        for (std::size_t i = 1; i <= size; i++) {
+        for (uint64_t i = 1; i <= size; i++) {
             fac[i] = (fac[i - 1] * i) % p;
         }
     }

From 21b277152c779ef7d9c5cad6a7e7cd249f51bc1a Mon Sep 17 00:00:00 2001
From: "piotr.idzik" <vil02@o2.pl>
Date: Fri, 19 May 2023 22:17:36 +0200
Subject: [PATCH 04/21] fix: remove p from the argument list of NCRModuloP::ncr

---
 math/ncr_modulo_p.cpp | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/math/ncr_modulo_p.cpp b/math/ncr_modulo_p.cpp
index b7bc1b30835..01272fff397 100644
--- a/math/ncr_modulo_p.cpp
+++ b/math/ncr_modulo_p.cpp
@@ -89,7 +89,7 @@ class NCRModuloP {
      * @params[in] the numbers 'n', 'r' and 'p'
      * @returns the value nCr % p
      */
-    int64_t ncr(const uint64_t& n, const uint64_t& r, const uint64_t& p) {
+    int64_t ncr(const uint64_t& n, const uint64_t& r) {
         // Base cases
         if (r > n) {
             return 0;
@@ -121,13 +121,19 @@ class NCRModuloP {
  * ncr function
  * @returns void
  */
-static void tests(math::ncr_modulo_p::NCRModuloP ncrObj) {
+static void tests() {
     // (52323 C 26161) % (1e9 + 7) = 224944353
-    assert(ncrObj.ncr(52323, 26161, 1000000007) == 224944353);
+    assert(
+        math::ncr_modulo_p::NCRModuloP(60000, 1000000007).ncr(52323, 26161) ==
+        224944353);
     // 6 C 2 = 30, 30%5 = 0
-    assert(ncrObj.ncr(6, 2, 5) == 0);
+    assert(math::ncr_modulo_p::NCRModuloP(20, 5).ncr(6, 2) == 0);
     // 7C3 = 35, 35 % 29 = 8
-    assert(ncrObj.ncr(7, 3, 29) == 6);
+    assert(math::ncr_modulo_p::NCRModuloP(100, 29).ncr(7, 3) == 6);
+}
+
+void other_tests() {
+    assert(math::ncr_modulo_p::NCRModuloP(1000, 13).ncr(10, 3) == 120 % 13);
 }
 
 /**
@@ -142,9 +148,10 @@ int main() {
         math::ncr_modulo_p::NCRModuloP(size, p);
     // test 6Ci for i=0 to 7
     for (int i = 0; i <= 7; i++) {
-        std::cout << 6 << "C" << i << " = " << ncrObj.ncr(6, i, p) << "\n";
+        std::cout << 6 << "C" << i << " = " << ncrObj.ncr(6, i) << "\n";
     }
-    tests(ncrObj);  // execute the tests
+    tests();  // execute the tests
+    other_tests();
     std::cout << "Assertions passed\n";
     return 0;
 }

From 69f48b918cac787e63d09fa1b0da84aa064335e1 Mon Sep 17 00:00:00 2001
From: "piotr.idzik" <vil02@o2.pl>
Date: Fri, 19 May 2023 22:21:58 +0200
Subject: [PATCH 05/21] refactor: add utils namespace

---
 math/ncr_modulo_p.cpp | 80 ++++++++++++++++++++++---------------------
 1 file changed, 41 insertions(+), 39 deletions(-)

diff --git a/math/ncr_modulo_p.cpp b/math/ncr_modulo_p.cpp
index 01272fff397..ed711423100 100644
--- a/math/ncr_modulo_p.cpp
+++ b/math/ncr_modulo_p.cpp
@@ -25,6 +25,45 @@ namespace math {
  * implementation.
  */
 namespace ncr_modulo_p {
+
+namespace utils {
+/** Finds the value of x, y such that a*x + b*y = gcd(a,b)
+ *
+ * @params[in] the numbers 'a', 'b' and address of 'x' and 'y' from above
+ * equation
+ * @returns the gcd of a and b
+ */
+uint64_t gcdExtended(const uint64_t& a, const uint64_t& b, int64_t* x,
+                     int64_t* y) {
+    if (a == 0) {
+        *x = 0, *y = 1;
+        return b;
+    }
+
+    int64_t x1 = 0, y1 = 0;
+    uint64_t gcd = gcdExtended(b % a, a, &x1, &y1);
+
+    *x = y1 - (b / a) * x1;
+    *y = x1;
+    return gcd;
+}
+
+/** Find modular inverse of a with m i.e. a number x such that (a*x)%m = 1
+ *
+ * @params[in] the numbers 'a' and 'm' from above equation
+ * @returns the modular inverse of a
+ */
+int64_t modInverse(const uint64_t& a, const uint64_t& m) {
+    int64_t x = 0, y = 0;
+    uint64_t g = gcdExtended(a, m, &x, &y);
+    if (g != 1) {  // modular inverse doesn't exist
+        return -1;
+    } else {
+        int64_t res = ((x + m) % m);
+        return res;
+    }
+}
+};  // namespace utils
 /**
  * @brief Class which contains all methods required for calculating nCr mod p
  */
@@ -47,43 +86,6 @@ class NCRModuloP {
         }
     }
 
-    /** Finds the value of x, y such that a*x + b*y = gcd(a,b)
-     *
-     * @params[in] the numbers 'a', 'b' and address of 'x' and 'y' from above
-     * equation
-     * @returns the gcd of a and b
-     */
-    uint64_t gcdExtended(const uint64_t& a, const uint64_t& b, int64_t* x,
-                         int64_t* y) {
-        if (a == 0) {
-            *x = 0, *y = 1;
-            return b;
-        }
-
-        int64_t x1 = 0, y1 = 0;
-        uint64_t gcd = gcdExtended(b % a, a, &x1, &y1);
-
-        *x = y1 - (b / a) * x1;
-        *y = x1;
-        return gcd;
-    }
-
-    /** Find modular inverse of a with m i.e. a number x such that (a*x)%m = 1
-     *
-     * @params[in] the numbers 'a' and 'm' from above equation
-     * @returns the modular inverse of a
-     */
-    int64_t modInverse(const uint64_t& a, const uint64_t& m) {
-        int64_t x = 0, y = 0;
-        uint64_t g = gcdExtended(a, m, &x, &y);
-        if (g != 1) {  // modular inverse doesn't exist
-            return -1;
-        } else {
-            int64_t res = ((x + m) % m);
-            return res;
-        }
-    }
-
     /** Find nCr % p
      *
      * @params[in] the numbers 'n', 'r' and 'p'
@@ -101,11 +103,11 @@ class NCRModuloP {
             return 1;
         }
         // fac is a global array with fac[r] = (r! % p)
-        int64_t denominator = modInverse(fac[r], p);
+        int64_t denominator = utils::modInverse(fac[r], p);
         if (denominator < 0) {  // modular inverse doesn't exist
             return -1;
         }
-        denominator = (denominator * modInverse(fac[n - r], p)) % p;
+        denominator = (denominator * utils::modInverse(fac[n - r], p)) % p;
         if (denominator < 0) {  // modular inverse doesn't exist
             return -1;
         }

From e3b71c5720126e60b41e135585adafe3d6c345ea Mon Sep 17 00:00:00 2001
From: "piotr.idzik" <vil02@o2.pl>
Date: Fri, 19 May 2023 22:23:26 +0200
Subject: [PATCH 06/21] refactor: use references in gcdExtended

---
 math/ncr_modulo_p.cpp | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/math/ncr_modulo_p.cpp b/math/ncr_modulo_p.cpp
index ed711423100..ae2383fcf63 100644
--- a/math/ncr_modulo_p.cpp
+++ b/math/ncr_modulo_p.cpp
@@ -33,18 +33,19 @@ namespace utils {
  * equation
  * @returns the gcd of a and b
  */
-uint64_t gcdExtended(const uint64_t& a, const uint64_t& b, int64_t* x,
-                     int64_t* y) {
+uint64_t gcdExtended(const uint64_t& a, const uint64_t& b, int64_t& x,
+                     int64_t& y) {
     if (a == 0) {
-        *x = 0, *y = 1;
+        x = 0;
+        y = 1;
         return b;
     }
 
     int64_t x1 = 0, y1 = 0;
-    uint64_t gcd = gcdExtended(b % a, a, &x1, &y1);
+    uint64_t gcd = gcdExtended(b % a, a, x1, y1);
 
-    *x = y1 - (b / a) * x1;
-    *y = x1;
+    x = y1 - (b / a) * x1;
+    y = x1;
     return gcd;
 }
 
@@ -55,7 +56,7 @@ uint64_t gcdExtended(const uint64_t& a, const uint64_t& b, int64_t* x,
  */
 int64_t modInverse(const uint64_t& a, const uint64_t& m) {
     int64_t x = 0, y = 0;
-    uint64_t g = gcdExtended(a, m, &x, &y);
+    uint64_t g = gcdExtended(a, m, x, y);
     if (g != 1) {  // modular inverse doesn't exist
         return -1;
     } else {

From 4672baa9231d56cc845a8c9fa1d452a6a2ba8d68 Mon Sep 17 00:00:00 2001
From: "piotr.idzik" <vil02@o2.pl>
Date: Fri, 19 May 2023 22:33:04 +0200
Subject: [PATCH 07/21] refactor: add NCRModuloP::computeFactorialsMod

---
 math/ncr_modulo_p.cpp | 32 ++++++++++++++++++++++----------
 1 file changed, 22 insertions(+), 10 deletions(-)

diff --git a/math/ncr_modulo_p.cpp b/math/ncr_modulo_p.cpp
index ae2383fcf63..ce53851077f 100644
--- a/math/ncr_modulo_p.cpp
+++ b/math/ncr_modulo_p.cpp
@@ -70,22 +70,34 @@ int64_t modInverse(const uint64_t& a, const uint64_t& m) {
  */
 class NCRModuloP {
  private:
-    std::vector<uint64_t> fac{};  /// stores precomputed factorial(i) % p value
-    uint64_t p = 0;               /// the p from (nCr % p)
+    const uint64_t p;  /// the p from (nCr % p)
+    const std::vector<uint64_t>
+        fac;  /// stores precomputed factorial(i) % p value
+
+    /**
+     * @brief computes the array of values of factorials reduced modulo mod
+     * @param max_arg_val argument of the last factorial stored in the result
+     * @param mod value of the divisor used to reduce factorials
+     * @return vector storing factorials of the numbers 0, ..., max_arg_val
+     * reduced modulo mod
+     */
+    static std::vector<uint64_t> computeFactorialsMod(
+        const uint64_t& max_arg_val, const uint64_t& mod) {
+        auto res = std::vector<uint64_t>(max_arg_val + 1);
+        res[0] = 1;
+        for (uint64_t i = 1; i <= max_arg_val; i++) {
+            res[i] = (res[i - 1] * i) % mod;
+        }
+        return res;
+    }
 
  public:
     /** Constructor which precomputes the values of n! % mod from n=0 to size
      *  and stores them in vector 'fac'
      *  @params[in] the numbers 'size', 'mod'
      */
-    NCRModuloP(const uint64_t& size, const uint64_t& mod) {
-        p = mod;
-        fac = std::vector<uint64_t>(size + 1);
-        fac[0] = 1;
-        for (uint64_t i = 1; i <= size; i++) {
-            fac[i] = (fac[i - 1] * i) % p;
-        }
-    }
+    NCRModuloP(const uint64_t& size, const uint64_t& mod)
+        : p(mod), fac(computeFactorialsMod(size, mod)) {}
 
     /** Find nCr % p
      *

From d712dc8b6170eb8890f4bb5658eaf80eb13491c4 Mon Sep 17 00:00:00 2001
From: "piotr.idzik" <vil02@o2.pl>
Date: Fri, 19 May 2023 22:34:06 +0200
Subject: [PATCH 08/21] style: make NCRModuloP::ncr const

---
 math/ncr_modulo_p.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/math/ncr_modulo_p.cpp b/math/ncr_modulo_p.cpp
index ce53851077f..9cbfa689663 100644
--- a/math/ncr_modulo_p.cpp
+++ b/math/ncr_modulo_p.cpp
@@ -104,7 +104,7 @@ class NCRModuloP {
      * @params[in] the numbers 'n', 'r' and 'p'
      * @returns the value nCr % p
      */
-    int64_t ncr(const uint64_t& n, const uint64_t& r) {
+    int64_t ncr(const uint64_t& n, const uint64_t& r) const {
         // Base cases
         if (r > n) {
             return 0;

From a13dff88d0bf4bc1c72d74b3ceccd4a70e2a6f35 Mon Sep 17 00:00:00 2001
From: "piotr.idzik" <vil02@o2.pl>
Date: Fri, 19 May 2023 22:42:54 +0200
Subject: [PATCH 09/21] test: reorganize tests

---
 math/ncr_modulo_p.cpp | 30 ++++++++++++++++++------------
 1 file changed, 18 insertions(+), 12 deletions(-)

diff --git a/math/ncr_modulo_p.cpp b/math/ncr_modulo_p.cpp
index 9cbfa689663..7f411189575 100644
--- a/math/ncr_modulo_p.cpp
+++ b/math/ncr_modulo_p.cpp
@@ -137,18 +137,25 @@ class NCRModuloP {
  * @returns void
  */
 static void tests() {
-    // (52323 C 26161) % (1e9 + 7) = 224944353
-    assert(
-        math::ncr_modulo_p::NCRModuloP(60000, 1000000007).ncr(52323, 26161) ==
-        224944353);
-    // 6 C 2 = 30, 30%5 = 0
-    assert(math::ncr_modulo_p::NCRModuloP(20, 5).ncr(6, 2) == 0);
-    // 7C3 = 35, 35 % 29 = 8
-    assert(math::ncr_modulo_p::NCRModuloP(100, 29).ncr(7, 3) == 6);
-}
+    struct TestCase {
+        const uint64_t size;
+        const uint64_t p;
+        const uint64_t n;
+        const uint64_t r;
+        const int64_t expected;
 
-void other_tests() {
-    assert(math::ncr_modulo_p::NCRModuloP(1000, 13).ncr(10, 3) == 120 % 13);
+        TestCase(const uint64_t size, const uint64_t p, const uint64_t n,
+                 const uint64_t r, const int64_t expected)
+            : size(size), p(p), n(n), r(r), expected(expected) {}
+    };
+    const std::vector<TestCase> test_cases = {
+        TestCase(60000, 1000000007, 52323, 26161, 224944353),
+        TestCase(20, 5, 6, 2, 30 % 5), TestCase(100, 29, 7, 3, 35 % 29),
+        TestCase(1000, 13, 10, 3, 120 % 13)};
+    for (const auto& tc : test_cases) {
+        assert(math::ncr_modulo_p::NCRModuloP(tc.size, tc.p).ncr(tc.n, tc.r) ==
+               tc.expected);
+    }
 }
 
 /**
@@ -166,7 +173,6 @@ int main() {
         std::cout << 6 << "C" << i << " = " << ncrObj.ncr(6, i) << "\n";
     }
     tests();  // execute the tests
-    other_tests();
     std::cout << "Assertions passed\n";
     return 0;
 }

From af191b57126efb630a4cdb54abeca522683d46c3 Mon Sep 17 00:00:00 2001
From: "piotr.idzik" <vil02@o2.pl>
Date: Fri, 19 May 2023 22:49:56 +0200
Subject: [PATCH 10/21] test: add missing test cases

---
 math/ncr_modulo_p.cpp | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/math/ncr_modulo_p.cpp b/math/ncr_modulo_p.cpp
index 7f411189575..4034736e375 100644
--- a/math/ncr_modulo_p.cpp
+++ b/math/ncr_modulo_p.cpp
@@ -150,8 +150,13 @@ static void tests() {
     };
     const std::vector<TestCase> test_cases = {
         TestCase(60000, 1000000007, 52323, 26161, 224944353),
-        TestCase(20, 5, 6, 2, 30 % 5), TestCase(100, 29, 7, 3, 35 % 29),
-        TestCase(1000, 13, 10, 3, 120 % 13)};
+        TestCase(20, 5, 6, 2, 30 % 5),
+        TestCase(100, 29, 7, 3, 35 % 29),
+        TestCase(1000, 13, 10, 3, 120 % 13),
+        TestCase(20, 17, 1, 10, 0),
+        TestCase(45, 19, 23, 1, 23 % 19),
+        TestCase(45, 19, 23, 0, 1),
+        TestCase(45, 19, 23, 23, 1)};
     for (const auto& tc : test_cases) {
         assert(math::ncr_modulo_p::NCRModuloP(tc.size, tc.p).ncr(tc.n, tc.r) ==
                tc.expected);

From d29c96f7501aaef9c95c6b918bad7596acb3c19e Mon Sep 17 00:00:00 2001
From: "piotr.idzik" <vil02@o2.pl>
Date: Fri, 19 May 2023 23:04:59 +0200
Subject: [PATCH 11/21] refactor: simplify logic

---
 math/ncr_modulo_p.cpp | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/math/ncr_modulo_p.cpp b/math/ncr_modulo_p.cpp
index 4034736e375..81ad88a99b7 100644
--- a/math/ncr_modulo_p.cpp
+++ b/math/ncr_modulo_p.cpp
@@ -116,15 +116,12 @@ class NCRModuloP {
             return 1;
         }
         // fac is a global array with fac[r] = (r! % p)
-        int64_t denominator = utils::modInverse(fac[r], p);
-        if (denominator < 0) {  // modular inverse doesn't exist
+        const auto denominator = (fac[r] * fac[n - r]) % p;
+        const auto denominator_inv = utils::modInverse(denominator, p);
+        if (denominator_inv < 0) {  // modular inverse doesn't exist
             return -1;
         }
-        denominator = (denominator * utils::modInverse(fac[n - r], p)) % p;
-        if (denominator < 0) {  // modular inverse doesn't exist
-            return -1;
-        }
-        return (fac[n] * denominator) % p;
+        return (fac[n] * denominator_inv) % p;
     }
 };
 }  // namespace ncr_modulo_p
@@ -156,7 +153,8 @@ static void tests() {
         TestCase(20, 17, 1, 10, 0),
         TestCase(45, 19, 23, 1, 23 % 19),
         TestCase(45, 19, 23, 0, 1),
-        TestCase(45, 19, 23, 23, 1)};
+        TestCase(45, 19, 23, 23, 1),
+        TestCase(20, 9, 10, 2, -1)};
     for (const auto& tc : test_cases) {
         assert(math::ncr_modulo_p::NCRModuloP(tc.size, tc.p).ncr(tc.n, tc.r) ==
                tc.expected);

From 334d91e554b9ff17639ea51696cc2a5812757b97 Mon Sep 17 00:00:00 2001
From: "piotr.idzik" <vil02@o2.pl>
Date: Fri, 19 May 2023 23:05:37 +0200
Subject: [PATCH 12/21] style: make example object const

---
 math/ncr_modulo_p.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/math/ncr_modulo_p.cpp b/math/ncr_modulo_p.cpp
index 81ad88a99b7..28c180cd484 100644
--- a/math/ncr_modulo_p.cpp
+++ b/math/ncr_modulo_p.cpp
@@ -169,7 +169,7 @@ int main() {
     // populate the fac array
     const uint64_t size = 1e6 + 1;
     const uint64_t p = 1e9 + 7;
-    math::ncr_modulo_p::NCRModuloP ncrObj =
+    const math::ncr_modulo_p::NCRModuloP ncrObj =
         math::ncr_modulo_p::NCRModuloP(size, p);
     // test 6Ci for i=0 to 7
     for (int i = 0; i <= 7; i++) {

From 478edd5cb4a85089bef9111f51acae44c8368f66 Mon Sep 17 00:00:00 2001
From: "piotr.idzik" <vil02@o2.pl>
Date: Fri, 19 May 2023 23:08:54 +0200
Subject: [PATCH 13/21] style: use auto

---
 math/ncr_modulo_p.cpp | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/math/ncr_modulo_p.cpp b/math/ncr_modulo_p.cpp
index 28c180cd484..cbb011853fc 100644
--- a/math/ncr_modulo_p.cpp
+++ b/math/ncr_modulo_p.cpp
@@ -169,8 +169,7 @@ int main() {
     // populate the fac array
     const uint64_t size = 1e6 + 1;
     const uint64_t p = 1e9 + 7;
-    const math::ncr_modulo_p::NCRModuloP ncrObj =
-        math::ncr_modulo_p::NCRModuloP(size, p);
+    const auto ncrObj = math::ncr_modulo_p::NCRModuloP(size, p);
     // test 6Ci for i=0 to 7
     for (int i = 0; i <= 7; i++) {
         std::cout << 6 << "C" << i << " = " << ncrObj.ncr(6, i) << "\n";

From 80b69dba6e80f9c7bc02234468d8b355242472d5 Mon Sep 17 00:00:00 2001
From: "piotr.idzik" <vil02@o2.pl>
Date: Sat, 20 May 2023 11:02:06 +0200
Subject: [PATCH 14/21] style: use int64_t to avoid narrowing conversions

---
 math/ncr_modulo_p.cpp | 45 +++++++++++++++++++++----------------------
 1 file changed, 22 insertions(+), 23 deletions(-)

diff --git a/math/ncr_modulo_p.cpp b/math/ncr_modulo_p.cpp
index cbb011853fc..942bc4568e2 100644
--- a/math/ncr_modulo_p.cpp
+++ b/math/ncr_modulo_p.cpp
@@ -33,8 +33,8 @@ namespace utils {
  * equation
  * @returns the gcd of a and b
  */
-uint64_t gcdExtended(const uint64_t& a, const uint64_t& b, int64_t& x,
-                     int64_t& y) {
+int64_t gcdExtended(const int64_t& a, const int64_t& b, int64_t& x,
+                    int64_t& y) {
     if (a == 0) {
         x = 0;
         y = 1;
@@ -42,7 +42,7 @@ uint64_t gcdExtended(const uint64_t& a, const uint64_t& b, int64_t& x,
     }
 
     int64_t x1 = 0, y1 = 0;
-    uint64_t gcd = gcdExtended(b % a, a, x1, y1);
+    const int64_t gcd = gcdExtended(b % a, a, x1, y1);
 
     x = y1 - (b / a) * x1;
     y = x1;
@@ -54,14 +54,13 @@ uint64_t gcdExtended(const uint64_t& a, const uint64_t& b, int64_t& x,
  * @params[in] the numbers 'a' and 'm' from above equation
  * @returns the modular inverse of a
  */
-int64_t modInverse(const uint64_t& a, const uint64_t& m) {
+int64_t modInverse(const int64_t& a, const int64_t& m) {
     int64_t x = 0, y = 0;
-    uint64_t g = gcdExtended(a, m, x, y);
+    const int64_t g = gcdExtended(a, m, x, y);
     if (g != 1) {  // modular inverse doesn't exist
         return -1;
     } else {
-        int64_t res = ((x + m) % m);
-        return res;
+        return ((x + m) % m);
     }
 }
 };  // namespace utils
@@ -70,8 +69,8 @@ int64_t modInverse(const uint64_t& a, const uint64_t& m) {
  */
 class NCRModuloP {
  private:
-    const uint64_t p;  /// the p from (nCr % p)
-    const std::vector<uint64_t>
+    const int64_t p;  /// the p from (nCr % p)
+    const std::vector<int64_t>
         fac;  /// stores precomputed factorial(i) % p value
 
     /**
@@ -81,11 +80,11 @@ class NCRModuloP {
      * @return vector storing factorials of the numbers 0, ..., max_arg_val
      * reduced modulo mod
      */
-    static std::vector<uint64_t> computeFactorialsMod(
-        const uint64_t& max_arg_val, const uint64_t& mod) {
-        auto res = std::vector<uint64_t>(max_arg_val + 1);
+    static std::vector<int64_t> computeFactorialsMod(const int64_t& max_arg_val,
+                                                     const int64_t& mod) {
+        auto res = std::vector<int64_t>(max_arg_val + 1);
         res[0] = 1;
-        for (uint64_t i = 1; i <= max_arg_val; i++) {
+        for (int64_t i = 1; i <= max_arg_val; i++) {
             res[i] = (res[i - 1] * i) % mod;
         }
         return res;
@@ -96,7 +95,7 @@ class NCRModuloP {
      *  and stores them in vector 'fac'
      *  @params[in] the numbers 'size', 'mod'
      */
-    NCRModuloP(const uint64_t& size, const uint64_t& mod)
+    NCRModuloP(const int64_t& size, const int64_t& mod)
         : p(mod), fac(computeFactorialsMod(size, mod)) {}
 
     /** Find nCr % p
@@ -104,7 +103,7 @@ class NCRModuloP {
      * @params[in] the numbers 'n', 'r' and 'p'
      * @returns the value nCr % p
      */
-    int64_t ncr(const uint64_t& n, const uint64_t& r) const {
+    int64_t ncr(const int64_t& n, const int64_t& r) const {
         // Base cases
         if (r > n) {
             return 0;
@@ -135,14 +134,14 @@ class NCRModuloP {
  */
 static void tests() {
     struct TestCase {
-        const uint64_t size;
-        const uint64_t p;
-        const uint64_t n;
-        const uint64_t r;
+        const int64_t size;
+        const int64_t p;
+        const int64_t n;
+        const int64_t r;
         const int64_t expected;
 
-        TestCase(const uint64_t size, const uint64_t p, const uint64_t n,
-                 const uint64_t r, const int64_t expected)
+        TestCase(const int64_t size, const int64_t p, const int64_t n,
+                 const int64_t r, const int64_t expected)
             : size(size), p(p), n(n), r(r), expected(expected) {}
     };
     const std::vector<TestCase> test_cases = {
@@ -167,8 +166,8 @@ static void tests() {
  */
 int main() {
     // populate the fac array
-    const uint64_t size = 1e6 + 1;
-    const uint64_t p = 1e9 + 7;
+    const int64_t size = 1e6 + 1;
+    const int64_t p = 1e9 + 7;
     const auto ncrObj = math::ncr_modulo_p::NCRModuloP(size, p);
     // test 6Ci for i=0 to 7
     for (int i = 0; i <= 7; i++) {

From 63e9d5b2f80dfe2cd7f51625547df198ca04803a Mon Sep 17 00:00:00 2001
From: "piotr.idzik" <vil02@o2.pl>
Date: Sat, 20 May 2023 11:14:43 +0200
Subject: [PATCH 15/21] docs: update explanation why to import iostream

---
 math/ncr_modulo_p.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/math/ncr_modulo_p.cpp b/math/ncr_modulo_p.cpp
index 942bc4568e2..5869fda83f5 100644
--- a/math/ncr_modulo_p.cpp
+++ b/math/ncr_modulo_p.cpp
@@ -10,7 +10,7 @@
  */
 
 #include <cassert>   /// for assert
-#include <iostream>  /// for io operations
+#include <iostream>  /// for std::cout
 #include <vector>    /// for std::vector
 
 /**

From e67ec3e62c159acd03cfe65d2ab06e95ed127a35 Mon Sep 17 00:00:00 2001
From: Piotr Idzik <65706193+vil02@users.noreply.github.com>
Date: Sun, 21 May 2023 17:15:33 +0200
Subject: [PATCH 16/21] docs: remove `p` from docstr of `NCRModuloP::ncr`

---
 math/ncr_modulo_p.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/math/ncr_modulo_p.cpp b/math/ncr_modulo_p.cpp
index 5869fda83f5..fd84ce1b855 100644
--- a/math/ncr_modulo_p.cpp
+++ b/math/ncr_modulo_p.cpp
@@ -100,7 +100,7 @@ class NCRModuloP {
 
     /** Find nCr % p
      *
-     * @params[in] the numbers 'n', 'r' and 'p'
+     * @params[in] the numbers 'n' and 'r'
      * @returns the value nCr % p
      */
     int64_t ncr(const int64_t& n, const int64_t& r) const {

From 94a475c88a1aeb450fa08f9e381ff06a7790049e Mon Sep 17 00:00:00 2001
From: "piotr.idzik" <vil02@o2.pl>
Date: Thu, 25 May 2023 22:33:36 +0200
Subject: [PATCH 17/21] docs: udpate doc-strs and add example()

---
 math/ncr_modulo_p.cpp | 73 +++++++++++++++++++++++++------------------
 1 file changed, 42 insertions(+), 31 deletions(-)

diff --git a/math/ncr_modulo_p.cpp b/math/ncr_modulo_p.cpp
index fd84ce1b855..ae7b3a384e3 100644
--- a/math/ncr_modulo_p.cpp
+++ b/math/ncr_modulo_p.cpp
@@ -26,12 +26,20 @@ namespace math {
  */
 namespace ncr_modulo_p {
 
+/**
+ * @namespace utils
+ * @brief this namespace contains the definitions of the functions called from
+ * the class NCRModuloP
+ */
 namespace utils {
-/** Finds the value of x, y such that a*x + b*y = gcd(a,b)
+/**
+ * @brief finds the values x and y such that a*x + b*y = gcd(a,b)
  *
- * @params[in] the numbers 'a', 'b' and address of 'x' and 'y' from above
- * equation
- * @returns the gcd of a and b
+ * @param[in] a the first input of the gcd
+ * @param[in] a the second input of the gcd
+ * @param[out] x the Bézout coefficient of a
+ * @param[out] y the Bézout coefficient of b
+ * @return the gcd of a and b
  */
 int64_t gcdExtended(const int64_t& a, const int64_t& b, int64_t& x,
                     int64_t& y) {
@@ -49,10 +57,11 @@ int64_t gcdExtended(const int64_t& a, const int64_t& b, int64_t& x,
     return gcd;
 }
 
-/** Find modular inverse of a with m i.e. a number x such that (a*x)%m = 1
+/** Find modular inverse of a modulo m i.e. a number x such that (a*x)%m = 1
  *
- * @params[in] the numbers 'a' and 'm' from above equation
- * @returns the modular inverse of a
+ * @param[in] a the number for which the modular inverse is queried
+ * @param[in] m the modulus
+ * @return the inverce of a modulo m, if it exists, -1 otherwise
  */
 int64_t modInverse(const int64_t& a, const int64_t& m) {
     int64_t x = 0, y = 0;
@@ -91,17 +100,18 @@ class NCRModuloP {
     }
 
  public:
-    /** Constructor which precomputes the values of n! % mod from n=0 to size
-     *  and stores them in vector 'fac'
-     *  @params[in] the numbers 'size', 'mod'
+    /**
+     * @brief constructs an NCRModuloP object allowing to compute (nCr)%p for
+     * inputs from 0 to size
      */
-    NCRModuloP(const int64_t& size, const int64_t& mod)
-        : p(mod), fac(computeFactorialsMod(size, mod)) {}
+    NCRModuloP(const int64_t& size, const int64_t& p)
+        : p(p), fac(computeFactorialsMod(size, p)) {}
 
-    /** Find nCr % p
-     *
-     * @params[in] the numbers 'n' and 'r'
-     * @returns the value nCr % p
+    /**
+     * @brief computes nCr % p
+     * @param[in] n the number of objects to be chosen
+     * @param[in] r the number of objects to choose from
+     * @return the value nCr % p
      */
     int64_t ncr(const int64_t& n, const int64_t& r) const {
         // Base cases
@@ -126,13 +136,7 @@ class NCRModuloP {
 }  // namespace ncr_modulo_p
 }  // namespace math
 
-/**
- * @brief Test implementations
- * @param ncrObj object which contains the precomputed factorial values and
- * ncr function
- * @returns void
- */
-static void tests() {
+void tests() {
     struct TestCase {
         const int64_t size;
         const int64_t p;
@@ -161,19 +165,26 @@ static void tests() {
 }
 
 /**
- * @brief Main function
- * @returns 0 on exit
+ * @brief example showing the usage of the NCRModuloP class
  */
-int main() {
-    // populate the fac array
+void example() {
     const int64_t size = 1e6 + 1;
     const int64_t p = 1e9 + 7;
+
+    // the ncrObj contains the precomputed values of factorials modulo p for
+    // values from 0 to size
     const auto ncrObj = math::ncr_modulo_p::NCRModuloP(size, p);
-    // test 6Ci for i=0 to 7
+
+    // having the ncrObj we can efficiently query the values of (n C r)%p
+    // note that time of the computation does not depend on size
     for (int i = 0; i <= 7; i++) {
-        std::cout << 6 << "C" << i << " = " << ncrObj.ncr(6, i) << "\n";
+        std::cout << 6 << "C" << i << " mod " << p << " = " << ncrObj.ncr(6, i)
+                  << "\n";
     }
-    tests();  // execute the tests
-    std::cout << "Assertions passed\n";
+}
+
+int main() {
+    tests();
+    example();
     return 0;
 }

From 6bf8ccf36382e363de148d1a0fcc53d5aa45dbfc Mon Sep 17 00:00:00 2001
From: Piotr Idzik <65706193+vil02@users.noreply.github.com>
Date: Tue, 30 May 2023 07:54:24 +0200
Subject: [PATCH 18/21] Apply suggestions from code review

Co-authored-by: David Leal <halfpacho@gmail.com>
---
 math/ncr_modulo_p.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/math/ncr_modulo_p.cpp b/math/ncr_modulo_p.cpp
index ae7b3a384e3..14ccf71d54a 100644
--- a/math/ncr_modulo_p.cpp
+++ b/math/ncr_modulo_p.cpp
@@ -72,7 +72,7 @@ int64_t modInverse(const int64_t& a, const int64_t& m) {
         return ((x + m) % m);
     }
 }
-};  // namespace utils
+}  // namespace utils
 /**
  * @brief Class which contains all methods required for calculating nCr mod p
  */
@@ -136,7 +136,7 @@ class NCRModuloP {
 }  // namespace ncr_modulo_p
 }  // namespace math
 
-void tests() {
+static void tests() {
     struct TestCase {
         const int64_t size;
         const int64_t p;

From c4d1f34c414282d0d3907cd84a923cb32b4c14a9 Mon Sep 17 00:00:00 2001
From: "piotr.idzik" <vil02@o2.pl>
Date: Wed, 21 Jun 2023 23:40:52 +0200
Subject: [PATCH 19/21] dosc: add missing docs

---
 math/ncr_modulo_p.cpp | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/math/ncr_modulo_p.cpp b/math/ncr_modulo_p.cpp
index 14ccf71d54a..ca46b4e6d32 100644
--- a/math/ncr_modulo_p.cpp
+++ b/math/ncr_modulo_p.cpp
@@ -29,7 +29,7 @@ namespace ncr_modulo_p {
 /**
  * @namespace utils
  * @brief this namespace contains the definitions of the functions called from
- * the class NCRModuloP
+ * the class math::ncr_modulo_p::NCRModuloP
  */
 namespace utils {
 /**
@@ -136,6 +136,9 @@ class NCRModuloP {
 }  // namespace ncr_modulo_p
 }  // namespace math
 
+/**
+ * @brief tests math::ncr_modulo_p::NCRModuloP
+ */
 static void tests() {
     struct TestCase {
         const int64_t size;
@@ -165,7 +168,7 @@ static void tests() {
 }
 
 /**
- * @brief example showing the usage of the NCRModuloP class
+ * @brief example showing the usage of the math::ncr_modulo_p::NCRModuloP class
  */
 void example() {
     const int64_t size = 1e6 + 1;

From 857e53dc387ac999a295f7ceed9bbd2969e4f540 Mon Sep 17 00:00:00 2001
From: Piotr Idzik <65706193+vil02@users.noreply.github.com>
Date: Fri, 23 Jun 2023 23:25:01 +0200
Subject: [PATCH 20/21] feat: display message when all tests pass

Co-authored-by: David Leal <halfpacho@gmail.com>
---
 math/ncr_modulo_p.cpp | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/math/ncr_modulo_p.cpp b/math/ncr_modulo_p.cpp
index ca46b4e6d32..766897ac18c 100644
--- a/math/ncr_modulo_p.cpp
+++ b/math/ncr_modulo_p.cpp
@@ -165,6 +165,8 @@ static void tests() {
         assert(math::ncr_modulo_p::NCRModuloP(tc.size, tc.p).ncr(tc.n, tc.r) ==
                tc.expected);
     }
+
+    std::cout << "\n\nAll tests have successfully passed!\n";
 }
 
 /**

From d44c2703234cc4e41bd5b1f402d23822da287f54 Mon Sep 17 00:00:00 2001
From: Piotr Idzik <65706193+vil02@users.noreply.github.com>
Date: Thu, 20 Jul 2023 09:22:28 +0200
Subject: [PATCH 21/21] style: initialize `NCRModuloP::p` with `0`

Co-authored-by: David Leal <halfpacho@gmail.com>
---
 math/ncr_modulo_p.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/math/ncr_modulo_p.cpp b/math/ncr_modulo_p.cpp
index 766897ac18c..cbf5e9e1a00 100644
--- a/math/ncr_modulo_p.cpp
+++ b/math/ncr_modulo_p.cpp
@@ -78,7 +78,7 @@ int64_t modInverse(const int64_t& a, const int64_t& m) {
  */
 class NCRModuloP {
  private:
-    const int64_t p;  /// the p from (nCr % p)
+    const int64_t p = 0;  /// the p from (nCr % p)
     const std::vector<int64_t>
         fac;  /// stores precomputed factorial(i) % p value