ronantakizawa/codereview-bench huggingface dataset:
Repository: TheAlgorithms/C-Plus-Plus
File: strings/duval.cpp (C++)

Code before:
    size_t i = 0, ans = 0;
    while (i < n) {
        ans = i;
        size_t j = i + 1, k = i;
        while (j < n + n && s[j % n] >= s[k % n]) {
            if (s[k % n] < s[j % n]) {
                k = i;
            } else {
                k++;
            }
            j++;
        }
        while (i <= k) {
            i += j - k;
        }
    }
    return ans;
    // returns 0-indexed position of the least cyclic shift
}

}  // namespace string

static void test() {
    using namespace string;

    // Test 1
    std::string s1 = "abcab";
    assert(duval(s1) == 3);

    // Test 2
    std::string s2 = "011100";
    assert(duval(s2) == 4);

    // Test 3
    std::vector<int> v = {5, 2, 1, 3, 4};
    assert(duval(v) == 2);

    // Test 4
    std::array<int, 5> a = {1, 2, 3, 4, 5};
    assert(duval(a) == 0);

    // Test 5
    std::deque<char> d = {'a', 'z', 'c', 'a', 'b'};
    assert(duval(d) == 3);

    std::cout << "All tests passed!" << std::endl;
}

int main() {
    test();
    return 0;

Review comment (suggestion):
```suggestion
/**
 * @brief self test implementation
 * returns void
 */
```


Code after:
 */
template <typename T>
size_t duval(const T& s) {
    size_t n = s.size();
    size_t i = 0, ans = 0;
    while (i < n) {
        ans = i;
        size_t j = i + 1, k = i;
        while (j < (n + n) && s[j % n] >= s[k % n]) {
            if (s[k % n] < s[j % n]) {
                k = i;
            } else {
                k++;
            }
            j++;
        }
        while (i <= k) {
            i += j - k;
        }
    }
    return ans;
    // returns 0-indexed position of the least cyclic shift
}

}  // namespace string

/**
 * @brief self test implementation
 * returns void
 */
static void test() {
    using namespace string;

    // Test 1
    std::string s1 = "abcab";
    assert(duval(s1) == 3);

    // Test 2
    std::string s2 = "011100";
    assert(duval(s2) == 4);

    // Test 3
    std::vector<int> v = {5, 2, 1, 3, 4};
    assert(duval(v) == 2);

    // Test 4
    std::array<int, 5> a = {1, 2, 3, 4, 5};
    assert(duval(a) == 0);

    // Test 5
    std::deque<char> d = {'a', 'z', 'c', 'a', 'b'};
Repository: TheAlgorithms/C-Plus-Plus
File: bit_manipulation/gray_code.cpp (C++)

Code before:
/**
 * @brief Program to generate n-bit [Gray 
 * code](https://en.wikipedia.org/wiki/Gray_code)
 * 
 * @details
 * Gray code is a binary numeral system
 * where consecutive values differ in exactly 1 bit.
 * The following code offers one of many possible Gray codes 
 * given some pre-determined number of bits.
 */

#include <bitset>  /// for gray code representation
#include <cassert>  /// for assert
#include <iostream>  /// for IO operations
#include <vector>  /// for vector data structure

/**
 * @brief Generate n-bit Gray code
 *
 * @param n Number of bits
 * @return A vector that stores the n-bit Gray code
 */

std::vector<std::bitset<32>> gray_code_generation(int n) {
    std::vector<std::bitset<32>> gray_code = {};  // Initialise empty vector
    
    // No Gray codes for non-positive values of n
    if (n <= 0) {
        return gray_code;
    }
    
    int total_codes = 1 << n;  // Number of n-bit gray codes

    for (int i = 0; i < total_codes; i++) {
        int gray_num = i ^ (i >> 1);  // Gray code formula
        gray_code.push_back(std::bitset<32>(gray_num));  // Store the value
    }

    return gray_code;
}


Review comment (suggestion):
```cpp
namespace bit_manipulation {
```
This should be inside a name space
Or maybe two 

Code after:
/**
 * @brief Program to generate n-bit [Gray 
 * code](https://en.wikipedia.org/wiki/Gray_code)
 * 
 * @details
 * Gray code is a binary numeral system
 * where consecutive values differ in exactly 1 bit.
 * The following code offers one of many possible Gray codes 
 * given some pre-determined number of bits.
 */

#include <bitset>  /// for gray code representation
#include <cassert>  /// for assert
#include <iostream>  /// for IO operations
#include <vector>  /// for vector data structure

/**
 * @namespace bit_manipulation
 * @brief Bit manipulation algorithms
 */
namespace bit_manipulation {
/**
 * @namespace gray_code
 * @brief Generate n-bit Gray code
 */
namespace gray_code {
/**
 * @brief The main function to generate n-bit Gray code
 *
 * @param n Number of bits
 * @return A vector that stores the n-bit Gray code
 */
std::vector<std::bitset<32>> gray_code_generation(int n) {
    std::vector<std::bitset<32>> gray_code = {};  // Initialise empty vector

    // No Gray codes for non-positive values of n
    if (n <= 0) {
        return gray_code;
    }
    
    int total_codes = 1 << n;  // Number of n-bit gray codes
Repository: TheAlgorithms/C-Plus-Plus
File: others/lfu_cache.cpp (C++)

Code before:
/**
 * @file
 * @brief Implementation for [LFU Cache]
 * (https://en.wikipedia.org/wiki/Least_frequently_used)
 *
 * @details
 * LFU discards the least frequently used value. if there are multiple items
 * with the same minimum frequency then, the least recently used among them is
 * discarded. Data structures used - doubly linked list and unordered_map(hash
 * map).
 *
 * Hashmap maps the key to the address of the node of the linked list and its
 * current usage frequency. If the element is accessed the element is removed
 * from the linked list of the current frequency and added to the linked list of
 * incremented frequency.
 *
 * When the cache is full, the last element in the minimum frequency linked list
 * is popped.
 *
 * @author [Karan Sharma](https://github.com/deDSeC00720)
 */

#include <cassert>        // for assert
#include <iostream>       // for IO operations
#include <unordered_map>  // for hash map

/**
 * @namespace
 * @brief Other algorithms
 */
namespace others {

/**
 * @namespace
 * @brief Cache algorithm
 */
namespace Cache {

/**
 * @class
 * @brief Node for a doubly linked list with data, prev and next pointers
 */
template <typename T>
class D_Node {
 public:
    T data;
    D_Node<T> *prev;
    D_Node<T> *next;

    explicit D_Node(T data) : data(data), prev(nullptr), next(nullptr) {}

Review comment (suggestion):
```suggestion
#include <iostream>       // for std::cout
#include <unordered_map>  // for std::unordered_map
```

Code after:
/**
 * @file
 * @brief Implementation for [LFU Cache]
 * (https://en.wikipedia.org/wiki/Least_frequently_used)
 *
 * @details
 * LFU discards the least frequently used value. if there are multiple items
 * with the same minimum frequency then, the least recently used among them is
 * discarded. Data structures used - doubly linked list and unordered_map(hash
 * map).
 *
 * Hashmap maps the key to the address of the node of the linked list and its
 * current usage frequency. If the element is accessed the element is removed
 * from the linked list of the current frequency and added to the linked list of
 * incremented frequency.
 *
 * When the cache is full, the last element in the minimum frequency linked list
 * is popped.
 *
 * @author [Karan Sharma](https://github.com/deDSeC00720)
 */

#include <cassert>        // for assert
#include <iostream>       // for std::cout
#include <unordered_map>  // for std::unordered_map

/**
 * @namespace
 * @brief Other algorithms
 */
namespace others {

/**
 * @namespace
 * @brief Cache algorithm
 */
namespace Cache {

/**
 * @class
 * @brief Node for a doubly linked list with data, prev and next pointers
 * @tparam T type of the data of the node
 */
template <typename T>
class D_Node {
 public:
    T data;           ///< data of the node
    D_Node<T> *prev;  ///< previous node in the doubly linked list
    D_Node<T> *next;  ///< next node in the doubly linked list
Repository: TheAlgorithms/C-Plus-Plus
File: others/lfu_cache.cpp (C++)

Code before:
 * When the cache is full, the last element in the minimum frequency linked list
 * is popped.
 *
 * @author [Karan Sharma](https://github.com/deDSeC00720)
 */

#include <cassert>        // for assert
#include <iostream>       // for IO operations
#include <unordered_map>  // for hash map

/**
 * @namespace
 * @brief Other algorithms
 */
namespace others {

/**
 * @namespace
 * @brief Cache algorithm
 */
namespace Cache {

/**
 * @class
 * @brief Node for a doubly linked list with data, prev and next pointers
 */
template <typename T>
class D_Node {
 public:
    T data;
    D_Node<T> *prev;
    D_Node<T> *next;

    explicit D_Node(T data) : data(data), prev(nullptr), next(nullptr) {}
};

template <typename K, typename V>
using CacheNode = D_Node<std::pair<K, V>>;

/**
 * @class
 * @brief LFUCache
 */
template <typename K, typename V>
class LFUCache {
    std::unordered_map<K, std::pair<CacheNode<K, V> *, int>>
        node_map;  ///< maps the key to the node address and frequency
    std::unordered_map<int, std::pair<CacheNode<K, V> *, CacheNode<K, V> *>>
        freq_map;  ///< maps the frequency to doubly linked list

    int minFreq;    ///< minimum frequency in the cache

Review comment (suggestion):
```suggestion
/**
 * @class
 * @brief Node for a doubly linked list with data, prev and next pointers
 * @tparam T type of the data of the node
 */
```

Code after:
 * When the cache is full, the last element in the minimum frequency linked list
 * is popped.
 *
 * @author [Karan Sharma](https://github.com/deDSeC00720)
 */

#include <cassert>        // for assert
#include <iostream>       // for std::cout
#include <unordered_map>  // for std::unordered_map

/**
 * @namespace
 * @brief Other algorithms
 */
namespace others {

/**
 * @namespace
 * @brief Cache algorithm
 */
namespace Cache {

/**
 * @class
 * @brief Node for a doubly linked list with data, prev and next pointers
 * @tparam T type of the data of the node
 */
template <typename T>
class D_Node {
 public:
    T data;           ///< data of the node
    D_Node<T> *prev;  ///< previous node in the doubly linked list
    D_Node<T> *next;  ///< next node in the doubly linked list

    explicit D_Node(T data) : data(data), prev(nullptr), next(nullptr) {}
};

template <typename K, typename V>
using CacheNode = D_Node<std::pair<K, V>>;

/**
 * @class
 * @brief LFUCache
 * @tparam K type of key in the LFU
 * @tparam V type of value in the LFU
 */
template <typename K, typename V>
class LFUCache {
    std::unordered_map<K, std::pair<CacheNode<K, V> *, int>>
        node_map;  ///< maps the key to the node address and frequency
    std::unordered_map<int, std::pair<CacheNode<K, V> *, CacheNode<K, V> *>>
Repository: TheAlgorithms/C-Plus-Plus
File: others/lfu_cache.cpp (C++)

Code before:
#include <cassert>        // for assert
#include <iostream>       // for IO operations
#include <unordered_map>  // for hash map

/**
 * @namespace
 * @brief Other algorithms
 */
namespace others {

/**
 * @namespace
 * @brief Cache algorithm
 */
namespace Cache {

/**
 * @class
 * @brief Node for a doubly linked list with data, prev and next pointers
 */
template <typename T>
class D_Node {
 public:
    T data;
    D_Node<T> *prev;
    D_Node<T> *next;

    explicit D_Node(T data) : data(data), prev(nullptr), next(nullptr) {}
};

template <typename K, typename V>
using CacheNode = D_Node<std::pair<K, V>>;

/**
 * @class
 * @brief LFUCache
 */
template <typename K, typename V>
class LFUCache {
    std::unordered_map<K, std::pair<CacheNode<K, V> *, int>>
        node_map;  ///< maps the key to the node address and frequency
    std::unordered_map<int, std::pair<CacheNode<K, V> *, CacheNode<K, V> *>>
        freq_map;  ///< maps the frequency to doubly linked list

    int minFreq;    ///< minimum frequency in the cache
    int _capacity;  ///< maximum capacity of the cache

 public:
    /**
     * @brief Constructor, Initialize with minFreq and _capacity.
     * @param _capacity Total capacity of the cache.

Review comment (suggestion):
```suggestion
    T data;           ///< data of the node
    D_Node<T> *prev;  ///< previous node in the doubly linked list
    D_Node<T> *next;  ///< next node in the doubly linked list
```

Code after:
#include <cassert>        // for assert
#include <iostream>       // for std::cout
#include <unordered_map>  // for std::unordered_map

/**
 * @namespace
 * @brief Other algorithms
 */
namespace others {

/**
 * @namespace
 * @brief Cache algorithm
 */
namespace Cache {

/**
 * @class
 * @brief Node for a doubly linked list with data, prev and next pointers
 * @tparam T type of the data of the node
 */
template <typename T>
class D_Node {
 public:
    T data;           ///< data of the node
    D_Node<T> *prev;  ///< previous node in the doubly linked list
    D_Node<T> *next;  ///< next node in the doubly linked list

    explicit D_Node(T data) : data(data), prev(nullptr), next(nullptr) {}
};

template <typename K, typename V>
using CacheNode = D_Node<std::pair<K, V>>;

/**
 * @class
 * @brief LFUCache
 * @tparam K type of key in the LFU
 * @tparam V type of value in the LFU
 */
template <typename K, typename V>
class LFUCache {
    std::unordered_map<K, std::pair<CacheNode<K, V> *, int>>
        node_map;  ///< maps the key to the node address and frequency
    std::unordered_map<int, std::pair<CacheNode<K, V> *, CacheNode<K, V> *>>
        freq_map;  ///< maps the frequency to doubly linked list

    int minFreq;    ///< minimum frequency in the cache
    int _capacity;  ///< maximum capacity of the cache

 public:
Repository: TheAlgorithms/C-Plus-Plus
File: others/lfu_cache.cpp (C++)

Code before:
 * @namespace
 * @brief Cache algorithm
 */
namespace Cache {

/**
 * @class
 * @brief Node for a doubly linked list with data, prev and next pointers
 */
template <typename T>
class D_Node {
 public:
    T data;
    D_Node<T> *prev;
    D_Node<T> *next;

    explicit D_Node(T data) : data(data), prev(nullptr), next(nullptr) {}
};

template <typename K, typename V>
using CacheNode = D_Node<std::pair<K, V>>;

/**
 * @class
 * @brief LFUCache
 */
template <typename K, typename V>
class LFUCache {
    std::unordered_map<K, std::pair<CacheNode<K, V> *, int>>
        node_map;  ///< maps the key to the node address and frequency
    std::unordered_map<int, std::pair<CacheNode<K, V> *, CacheNode<K, V> *>>
        freq_map;  ///< maps the frequency to doubly linked list

    int minFreq;    ///< minimum frequency in the cache
    int _capacity;  ///< maximum capacity of the cache

 public:
    /**
     * @brief Constructor, Initialize with minFreq and _capacity.
     * @param _capacity Total capacity of the cache.
     */
    explicit LFUCache(int _capacity) : minFreq(0), _capacity(_capacity) {}

 private:
    /**
     * @brief push the node at first position in the linked list of given
     * frequency
     * @param freq the frequency mapping to the linked list where node should be
     * pushed.
     * @param node node to be pushed to the linked list.
     */

Review comment (performance):
```suggestion
/**
 * @class
 * @brief LFUCache
 * @tparam K type of the key in the LFU
 * @tparam V type of the value in the LFU
 */
```

Code after:
 * @namespace
 * @brief Cache algorithm
 */
namespace Cache {

/**
 * @class
 * @brief Node for a doubly linked list with data, prev and next pointers
 * @tparam T type of the data of the node
 */
template <typename T>
class D_Node {
 public:
    T data;           ///< data of the node
    D_Node<T> *prev;  ///< previous node in the doubly linked list
    D_Node<T> *next;  ///< next node in the doubly linked list

    explicit D_Node(T data) : data(data), prev(nullptr), next(nullptr) {}
};

template <typename K, typename V>
using CacheNode = D_Node<std::pair<K, V>>;

/**
 * @class
 * @brief LFUCache
 * @tparam K type of key in the LFU
 * @tparam V type of value in the LFU
 */
template <typename K, typename V>
class LFUCache {
    std::unordered_map<K, std::pair<CacheNode<K, V> *, int>>
        node_map;  ///< maps the key to the node address and frequency
    std::unordered_map<int, std::pair<CacheNode<K, V> *, CacheNode<K, V> *>>
        freq_map;  ///< maps the frequency to doubly linked list

    int minFreq;    ///< minimum frequency in the cache
    int _capacity;  ///< maximum capacity of the cache

 public:
    /**
     * @brief Constructor, Initialize with minFreq and _capacity.
     * @param _capacity Total capacity of the cache.
     */
    explicit LFUCache(int _capacity) : minFreq(0), _capacity(_capacity) {}

 private:
    /**
     * @brief push the node at first position in the linked list of given
     * frequency
     * @param freq the frequency mapping to the linked list where node should be
Repository: TheAlgorithms/C-Plus-Plus
File: others/lfu_cache.cpp (C++)

Code before:

/**
 * @brief A simple test case
 * @return void
 */
void test2() {
    others::Cache::LFUCache<int, int> cache(5);
    cache.put(-1, -10);
    cache.put(2, 20);
    cache.put(3, 30);

    assert(cache.get(-1) == -10);
    cache.get(2);
    cache.get(3);

    cache.put(4, 40);
    cache.put(5, 50);
    cache.put(6, 60);

    assert(cache.get(5) == 50);

    std::cout << "test2 - passed\n";
}

/**
 * @brief test method with 2 tests
 * @return void
 */
void test() {
    test1();
    test2();
}

/**
 * @brief main function
 * @return 0 on exit
 */
int main() {
    test();
    return 0;
}


Review comment (suggestion):
```suggestion
 * @brief self test implementation
```

Code after:

    cache.put(-3, -30);
    cache.put(4, 40);
    cache.put(5, -50);
    cache.put(6, 60);

    // test the state after inserting more items than the capacity
    assert(cache.size() == 5);
    assert(cache.capacity() == 5);
    assert(!cache.empty());

    // test retrieval of all items in the cache
    assert(cache.get(1) == 10);
    assert(cache.get(-2) == 20);

    // fetching -3 throws runtime_error
    // as -3 was evicted being the least frequently used
    // when 6 was added
    // assert(cache.get(-3) == -30);

    assert(cache.get(4) == 40);
    assert(cache.get(5) == -50);
    assert(cache.get(6) == 60);

    std::cout << "test - passed\n";
}

/**
 * @brief main function
 * @return 0 on exit
 */
int main() {
    test();  // run the self test implementation
    return 0;
}
Repository: TheAlgorithms/C-Plus-Plus
File: others/lfu_cache.cpp (C++)

Code before:
 */
void test2() {
    others::Cache::LFUCache<int, int> cache(5);
    cache.put(-1, -10);
    cache.put(2, 20);
    cache.put(3, 30);

    assert(cache.get(-1) == -10);
    cache.get(2);
    cache.get(3);

    cache.put(4, 40);
    cache.put(5, 50);
    cache.put(6, 60);

    assert(cache.get(5) == 50);

    std::cout << "test2 - passed\n";
}

/**
 * @brief self test implementation
 * @return void
 */
void test() {
    test1();
    test2();
}

/**
 * @brief main function
 * @return 0 on exit
 */
int main() {
    test();  // run the self test implementation
    return 0;
}


Review comment (suggestion):
```suggestion
    tests();  // run the self test implementation
```

Code after:
    assert(cache.size() == 5);
    assert(cache.capacity() == 5);
    assert(!cache.empty());

    // test retrieval of all items in the cache
    assert(cache.get(1) == 10);
    assert(cache.get(-2) == 20);

    // fetching -3 throws runtime_error
    // as -3 was evicted being the least frequently used
    // when 6 was added
    // assert(cache.get(-3) == -30);

    assert(cache.get(4) == 40);
    assert(cache.get(5) == -50);
    assert(cache.get(6) == 60);

    std::cout << "test - passed\n";
}

/**
 * @brief main function
 * @return 0 on exit
 */
int main() {
    test();  // run the self test implementation
    return 0;
}
Repository: TheAlgorithms/C-Plus-Plus
File: greedy_algorithms/digit_separation.cpp (C++)

Code before:
    expectedReverse = {5};
    expectedForward = {5};

    reverseOrder = ds.digitSeparationReverseOrder(number);
    assert(reverseOrder == expectedReverse);

    forwardOrder = ds.digitSeparationForwardOrder(number);
    assert(forwardOrder == expectedForward);

    // Test case: Zero
    number = 0;
    expectedReverse = {0};
    expectedForward = {0};

    reverseOrder = ds.digitSeparationReverseOrder(number);
    assert(reverseOrder == expectedReverse);

    forwardOrder = ds.digitSeparationForwardOrder(number);
    assert(forwardOrder == expectedForward);

    // Test case: Large number
    number = 987654321012345;
    expectedReverse = {5, 4, 3, 2, 1, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9};
    expectedForward = {9, 8, 7, 6, 5, 4, 3, 2, 1, 0, 1, 2, 3, 4, 5};

    reverseOrder = ds.digitSeparationReverseOrder(number);
    assert(reverseOrder == expectedReverse);

    forwardOrder = ds.digitSeparationForwardOrder(number);
    assert(forwardOrder == expectedForward);

    // Test case: Negative number
    number = -987654321012345;
    expectedReverse = {5, 4, 3, 2, 1, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9};
    expectedForward = {9, 8, 7, 6, 5, 4, 3, 2, 1, 0, 1, 2, 3, 4, 5};

    std::cout << "All tests passed!" << std::endl;

    return 0;
}


Review comment (suggestion):
```suggestion

    reverseOrder = ds.digitSeparationReverseOrder(number);
    assert(reverseOrder == expectedReverse);
```

Code after:
    expectedReverse = {0};
    expectedForward = {0};
    reverseOrder = ds.digitSeparationReverseOrder(number);
    assert(reverseOrder == expectedReverse);
    forwardOrder = ds.digitSeparationForwardOrder(number);
    assert(forwardOrder == expectedForward);

    // Test case: Large number
    number = 987654321012345;
    expectedReverse = {5, 4, 3, 2, 1, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9};
    expectedForward = {9, 8, 7, 6, 5, 4, 3, 2, 1, 0, 1, 2, 3, 4, 5};
    reverseOrder = ds.digitSeparationReverseOrder(number);
    assert(reverseOrder == expectedReverse);
    forwardOrder = ds.digitSeparationForwardOrder(number);
    assert(forwardOrder == expectedForward);

    // Test case: Negative number
    number = -987654321012345;
    expectedReverse = {5, 4, 3, 2, 1, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9};
    expectedForward = {9, 8, 7, 6, 5, 4, 3, 2, 1, 0, 1, 2, 3, 4, 5};
    reverseOrder = ds.digitSeparationReverseOrder(number);
    assert(reverseOrder == expectedReverse);
    forwardOrder = ds.digitSeparationForwardOrder(number);
    assert(forwardOrder == expectedForward);
}

/**
 * @brief main function
 * @return 0 on successful exit
 */
int main() {
    tests();  // run self test implementation

    return 0;
}