randi Template Function Implementation for Image in C++ (Rev.3)

Question

This is a follow-up question for An Updated Multi-dimensional Image Data Structure with Variadic Template Functions in C++, rand Template Function Implementation for Image in C++, randi Template Function Implementation for Image in C++ and randi Template Function Implementation for Image in C++ (Rev.2). As G. Sliepen's answer mentioned:

I also recommend that you have one overload of randi() that calls generate(), the rest just calls that overload, either directly or indirectly.

I followed the guidance and randi template function end up with the following form:

namespace TinyDIP
{
    //  randi template function implementation
    //  function that can handle everything, this one calls `generate()`
    template<std::integral ElementT = int, typename Urbg, std::same_as<std::size_t>... Sizes>
    requires std::uniform_random_bit_generator<std::remove_reference_t<Urbg>>
    constexpr static auto randi(Urbg&& urbg, std::pair<ElementT, ElementT> min_and_max, Sizes... sizes)
    {
        auto dist = std::uniform_int_distribution<ElementT>{ min_and_max.first, min_and_max.second };
        if constexpr (sizeof...(Sizes) == 0)
        {
            return generate([&dist, &urbg]() { return dist(urbg); }, std::size_t{ 1 });
        }
        else
        {
            return generate([&dist, &urbg]() { return dist(urbg); }, sizes...);
        }
    }
    
    // randi template function implementation
    template<std::integral ElementT = int, std::same_as<std::size_t>... Size>
    inline auto randi(std::pair<ElementT, ElementT> min_and_max, Size... size)
    {
        return randi<ElementT>(std::mt19937{std::random_device{}()}, min_and_max, size...);
    }
    
    // randi template function implementation
    template<std::integral ElementT = int, std::same_as<std::size_t>... Size>
    inline auto randi(ElementT max, Size... size)
    {
        return randi<ElementT>(std::mt19937{ std::random_device{}() }, std::pair<ElementT, ElementT>{static_cast<ElementT>(1), max}, size...);
    }
    
    //  randi template function implementation
    template<std::integral ElementT = int, typename Urbg>
    requires std::uniform_random_bit_generator<std::remove_reference_t<Urbg>>
    constexpr auto randi(Urbg&& urbg, ElementT max)
    {
        return randi<ElementT>(std::forward<Urbg>(urbg), std::pair<ElementT, ElementT>{static_cast<ElementT>(1), max});
    }
    
    // randi template function implementation
    template<std::integral ElementT = int>
    inline auto randi(ElementT max)
    {
        return randi<ElementT>(std::mt19937{std::random_device{}()}, max);
    }
}

The first one is the function that can handle everything; this one calls generate() and for handling the case which Sizes... sizes is empty, if constexpr (sizeof...(Sizes) == 0) is used here.

The current version generate template function:

namespace TinyDIP
{
    //  generate template function implementation
    template<std::ranges::input_range Sizes, typename F>
    requires((std::same_as<std::ranges::range_value_t<Sizes>, std::size_t>) and
             (std::invocable<F&>))
    constexpr static auto generate(F gen, const Sizes& sizes)
    {
        using ElementT = std::invoke_result_t<F>;
        auto count = std::reduce(std::ranges::cbegin(sizes), std::ranges::cend(sizes), 1, std::multiplies());
        std::vector<ElementT> element_vector(count);
        std::ranges::generate(element_vector, gen);
        Image<ElementT> image(element_vector, sizes);
        return image;
    }
    
    //  generate template function implementation
    //  https://codereview.stackexchange.com/a/295600/231235
    template<typename F, std::same_as<std::size_t>... Sizes>
    requires std::invocable<F&>
    constexpr static auto generate(F gen, Sizes... sizes)
    {
        return generate(gen, std::array{sizes...});
    }
}

The current version Image class:

namespace TinyDIP
{
    template <typename ElementT>
    class Image
    {
    public:
        Image() = default;

        template<std::same_as<std::size_t>... Sizes>
        Image(Sizes... sizes): size{sizes...}, image_data((1 * ... * sizes))
        {}

        template<std::same_as<int>... Sizes>
        Image(Sizes... sizes)
        {
            size.reserve(sizeof...(sizes));
            (size.emplace_back(sizes), ...);
            image_data.resize(
                std::reduce(
                    std::ranges::cbegin(size),
                    std::ranges::cend(size),
                    std::size_t{1},
                    std::multiplies<>()
                    )
            );
        }

        //  Image constructor
        template<std::ranges::input_range Sizes>
        requires(std::same_as<std::ranges::range_value_t<Sizes>, std::size_t>)
        Image(const Sizes& sizes)
        {
            if (sizes.empty())
            {
                throw std::runtime_error("Image size vector is empty!");
            }
            size.resize(sizes.size());
            std::transform(std::ranges::cbegin(sizes), std::ranges::cend(sizes), std::ranges::begin(size), [&](auto&& element) { return element; });
            image_data.resize(
                std::reduce(
                    std::ranges::cbegin(sizes),
                    std::ranges::cend(sizes),
                    std::size_t{1},
                    std::multiplies<>()
                    ));
        }

        //  Image constructor
        #ifdef __cpp_lib_containers_ranges
            template<std::ranges::input_range Range,
                     std::same_as<std::size_t>... Sizes>
            Image(const Range&& input, Sizes... sizes):
                size{sizes...}, image_data(std::from_range_t, std::forward<Range>(input))
            {
                if (image_data.size() != (1 * ... * sizes)) {
                    throw std::runtime_error("Image data input and the given size are mismatched!");
                }
            }
        #else
            template<std::ranges::input_range Range,
                     std::same_as<std::size_t>... Sizes>
            Image(const Range&& input, Sizes... sizes):
                size{sizes...}, image_data(input.begin(), input.end())
            {
                if (image_data.size() != (1 * ... * sizes)) {
                    throw std::runtime_error("Image data input and the given size are mismatched!");
                }
            }
        #endif

        //  Image constructor
        #ifdef __cpp_lib_containers_ranges
            template<std::ranges::input_range Range, std::same_as<std::size_t>... Sizes>
            Image(const Range& input, Sizes... sizes) :
                size{ sizes... }
            {
                if (input.empty())
                {
                    throw std::runtime_error("Input vector is empty!");
                }
                image_data = std::vector(std::from_range_t, input);
                if (image_data.size() != (1 * ... * sizes)) {
                    throw std::runtime_error("Image data input and the given size are mismatched!");
                }
            }
        #else
            template<std::ranges::input_range Range, std::same_as<std::size_t>... Sizes>
            Image(const Range& input, Sizes... sizes):
                size{sizes...}
            {
                if (input.empty())
                {
                    throw std::runtime_error("Input vector is empty!");
                }
                image_data = std::vector(input.begin(), input.end());
                if (image_data.size() != (1 * ... * sizes)) {
                    throw std::runtime_error("Image data input and the given size are mismatched!");
                }
            }
        #endif

        //  Image constructor
        template<std::ranges::input_range Range, std::ranges::input_range Sizes>
        requires(std::same_as<std::ranges::range_value_t<Sizes>, std::size_t>)
        Image(const Range& input, const Sizes& sizes)
        {
            if (input.empty())
            {
                throw std::runtime_error("Input vector is empty!");
            }
            size.resize(sizes.size());
            std::transform(std::ranges::cbegin(sizes), std::ranges::cend(sizes), std::ranges::begin(size), [&](auto&& element) { return element; });
            image_data = std::vector(std::ranges::cbegin(input), std::ranges::cend(input));
            auto count = std::reduce(std::ranges::cbegin(sizes), std::ranges::cend(sizes), 1, std::multiplies());
            if (image_data.size() != count) {
                throw std::runtime_error("Image data input and the given size are mismatched!");
            }
        }

        Image(const std::vector<std::vector<ElementT>>& input)
        {
            if (input.empty())
            {
                throw std::runtime_error("Input vector is empty!");
            }
            size.reserve(2);
            size.emplace_back(input[0].size());
            size.emplace_back(input.size());
            for (auto& rows : input)
            {
                image_data.insert(image_data.end(), std::ranges::begin(rows), std::ranges::end(rows));    //  flatten
            }
            return;
        }

        //  at template function implementation
        template<typename... Args>
        constexpr ElementT& at(const Args... indexInput)
        {
            return const_cast<ElementT&>(static_cast<const Image &>(*this).at(indexInput...));
        }

        //  at template function implementation
        //  Reference: https://codereview.stackexchange.com/a/288736/231235
        template<typename... Args>
        constexpr ElementT const& at(const Args... indexInput) const
        {
            checkBoundary(indexInput...);
            constexpr std::size_t n = sizeof...(Args);
            if(n != size.size())
            {
                throw std::runtime_error("Dimensionality mismatched!");
            }
            std::size_t i = 0;
            std::size_t stride = 1;
            std::size_t position = 0;

            auto update_position = [&](auto index) {
                position += index * stride;
                stride *= size[i++];
            };
            (update_position(indexInput), ...);

            return image_data[position];
        }

        //  at_without_boundary_check template function implementation
        template<typename... Args>
        constexpr ElementT& at_without_boundary_check(const Args... indexInput)
        {
            return const_cast<ElementT&>(static_cast<const Image &>(*this).at_without_boundary_check(indexInput...));
        }

        template<typename... Args>
        constexpr ElementT const& at_without_boundary_check(const Args... indexInput) const
        {
            std::size_t i = 0;
            std::size_t stride = 1;
            std::size_t position = 0;

            auto update_position = [&](auto index) {
                position += index * stride;
                stride *= size[i++];
            };
            (update_position(indexInput), ...);

            return image_data[position];
        }

        //  get function implementation
        constexpr ElementT get(std::size_t index) const noexcept
        {
            return image_data[index];
        }

        //  set function implementation
        constexpr ElementT& set(const std::size_t index) noexcept
        {
            return image_data[index];
        }

        //  set template function implementation
        template<class TupleT>
        requires(is_tuple<TupleT>::value and
                 check_tuple_element_type<std::size_t, TupleT>::value)
        constexpr bool set(const TupleT location, const ElementT draw_value)
        {
            if (checkBoundaryTuple(location))
            {
                image_data[tuple_location_to_index(location)] = draw_value;
                return true;
            }
            return false;
        }

        //  cast template function implementation
        template<typename TargetT>
        constexpr Image<TargetT> cast()
        {
            std::vector<TargetT> output_data;
            output_data.resize(image_data.size());
            std::transform(
                std::ranges::cbegin(image_data),
                std::ranges::cend(image_data),
                std::ranges::begin(output_data),
                [](auto& input){ return static_cast<TargetT>(input); }
                );
            Image<TargetT> output(output_data, size);
            return output;
        }

        constexpr std::size_t count() const noexcept
        {
            return std::reduce(std::ranges::cbegin(size), std::ranges::cend(size), std::size_t{ 1 }, std::multiplies());
        }

        //  count member function implementation
        template<class ExPo>
        requires (std::is_execution_policy_v<std::remove_cvref_t<ExPo>>)
        constexpr std::size_t count(ExPo execution_policy)
        {
            return std::reduce(execution_policy, std::ranges::cbegin(size), std::ranges::cend(size), std::size_t{ 1 }, std::multiplies());
        }
  
        constexpr std::size_t getDimensionality() const noexcept
        {
            return size.size();
        }

        constexpr std::size_t getWidth() const noexcept
        {
            return size[0];
        }

        constexpr std::size_t getHeight() const noexcept
        {
            return size[1];
        }

        //  getSize function implementation
        constexpr auto getSize() const noexcept
        {
            return size;
        }

        //  getSize function implementation
        constexpr auto getSize(std::size_t index) const noexcept
        {
            return size[index];
        }

        //  getStride function implementation
        constexpr std::size_t getStride(std::size_t index) const noexcept
        {
            if(index == 0)
            {
                return std::size_t{1};
            }
            std::size_t output = std::size_t{1};
            for(std::size_t i = 0; i < index; ++i)
            {
                output *= size[i];
            }
            return output;
        }

        std::vector<ElementT> const& getImageData() const noexcept { return image_data; }      //  expose the internal data

        //  print function implementation
        void print(std::string separator = "\t", std::ostream& os = std::cout) const
        {
            if constexpr (is_MultiChannel<ElementT>::value)
            {
                if (size.size() == 1)
                {
                    for (std::size_t x = 0; x < size[0]; ++x)
                    {
                        os << at(x) << separator;
                    }
                    os << "\n";
                }
                else if (size.size() == 2)
                {
                    for (std::size_t y = 0; y < size[1]; ++y)
                    {
                        for (std::size_t x = 0; x < size[0]; ++x)
                        {
                            os << at(x, y) << separator;
                        }
                        os << "\n";
                    }
                    os << "\n";
                }
                else if (size.size() == 3)
                {
                    for (std::size_t z = 0; z < size[2]; ++z)
                    {
                        for (std::size_t y = 0; y < size[1]; ++y)
                        {
                            for (std::size_t x = 0; x < size[0]; ++x)
                            {
                                os << at(x, y, z) << separator;
                            }
                            os << "\n";
                        }
                        os << "\n";
                    }
                    os << "\n";
                }
                else if (size.size() == 4)
                {
                    for (std::size_t a = 0; a < size[3]; ++a)
                    {
                        os << "group = " << a << "\n";
                        for (std::size_t z = 0; z < size[2]; ++z)
                        {
                            for (std::size_t y = 0; y < size[1]; ++y)
                            {
                                for (std::size_t x = 0; x < size[0]; ++x)
                                {
                                    os << at(x, y, z, a) << separator;
                                }
                                os << "\n";
                            }
                            os << "\n";
                        }
                        os << "\n";
                    }
                    os << "\n";
                }
            }
            else
            {
                if (size.size() == 1)
                {
                    for (std::size_t x = 0; x < size[0]; ++x)
                    {
                        //  Ref: https://isocpp.org/wiki/faq/input-output#print-char-or-ptr-as-number
                        os << +at(x) << separator;
                    }
                    os << "\n";
                }
                else if (size.size() == 2)
                {
                    for (std::size_t y = 0; y < size[1]; ++y)
                    {
                        for (std::size_t x = 0; x < size[0]; ++x)
                        {
                            //  Ref: https://isocpp.org/wiki/faq/input-output#print-char-or-ptr-as-number
                            os << +at(x, y) << separator;
                        }
                        os << "\n";
                    }
                    os << "\n";
                }
                else if (size.size() == 3)
                {
                    for (std::size_t z = 0; z < size[2]; ++z)
                    {
                        for (std::size_t y = 0; y < size[1]; ++y)
                        {
                            for (std::size_t x = 0; x < size[0]; ++x)
                            {
                                //  Ref: https://isocpp.org/wiki/faq/input-output#print-char-or-ptr-as-number
                                os << +at(x, y, z) << separator;
                            }
                            os << "\n";
                        }
                        os << "\n";
                    }
                    os << "\n";
                }
                else if (size.size() == 4)
                {
                    for (std::size_t a = 0; a < size[3]; ++a)
                    {
                        os << "group = " << a << "\n";
                        for (std::size_t z = 0; z < size[2]; ++z)
                        {
                            for (std::size_t y = 0; y < size[1]; ++y)
                            {
                                for (std::size_t x = 0; x < size[0]; ++x)
                                {
                                    //  Ref: https://isocpp.org/wiki/faq/input-output#print-char-or-ptr-as-number
                                    os << +at(x, y, z, a) << separator;
                                }
                                os << "\n";
                            }
                            os << "\n";
                        }
                        os << "\n";
                    }
                    os << "\n";
                }
            }
        }

        //  Enable this function if ElementT = RGB or RGB_DOUBLE or HSV
        void print(std::string separator = "\t", std::ostream& os = std::cout) const
        requires(std::same_as<ElementT, RGB> or std::same_as<ElementT, RGB_DOUBLE> or std::same_as<ElementT, HSV> or is_MultiChannel<ElementT>::value)
        {
            if (size.size() == 1)
            {
                for (std::size_t x = 0; x < size[0]; ++x)
                {
                    os << "( ";
                    for (std::size_t channel_index = 0; channel_index < 3; ++channel_index)
                    {
                        //  Ref: https://isocpp.org/wiki/faq/input-output#print-char-or-ptr-as-number
                        os << +at(x).channels[channel_index] << separator;
                    }
                    os << ")" << separator;
                }
                os << "\n";
            }
            else if (size.size() == 2)
            {
                for (std::size_t y = 0; y < size[1]; ++y)
                {
                    for (std::size_t x = 0; x < size[0]; ++x)
                    {
                        os << "( ";
                        for (std::size_t channel_index = 0; channel_index < 3; ++channel_index)
                        {
                            //  Ref: https://isocpp.org/wiki/faq/input-output#print-char-or-ptr-as-number
                            os << +at(x, y).channels[channel_index] << separator;
                        }
                        os << ")" << separator;
                    }
                    os << "\n";
                }
                os << "\n";
            }
            return;
        }

        Image<ElementT>& setAllValue(const ElementT input)
        {
            std::fill(std::ranges::begin(image_data), std::ranges::end(image_data), input);
            return *this;
        }

        friend std::ostream& operator<<(std::ostream& os, const Image<ElementT>& rhs)
        {
            const std::string separator = "\t";
            rhs.print(separator, os);
            return os;
        }

        Image<ElementT>& operator+=(const Image<ElementT>& rhs)
        {
            check_size_same(rhs, *this);
            std::transform(std::ranges::cbegin(image_data), std::ranges::cend(image_data), std::ranges::cbegin(rhs.image_data),
                   std::ranges::begin(image_data), std::plus<>{});
            return *this;
        }

        Image<ElementT>& operator-=(const Image<ElementT>& rhs)
        {
            check_size_same(rhs, *this);
            std::transform(std::ranges::cbegin(image_data), std::ranges::cend(image_data), std::ranges::cbegin(rhs.image_data),
                   std::ranges::begin(image_data), std::minus<>{});
            return *this;
        }

        Image<ElementT>& operator*=(const Image<ElementT>& rhs)
        {
            check_size_same(rhs, *this);
            std::transform(std::ranges::cbegin(image_data), std::ranges::cend(image_data), std::ranges::cbegin(rhs.image_data),
                   std::ranges::begin(image_data), std::multiplies<>{});
            return *this;
        }

        Image<ElementT>& operator/=(const Image<ElementT>& rhs)
        {
            check_size_same(rhs, *this);
            std::transform(std::ranges::cbegin(image_data), std::ranges::cend(image_data), std::ranges::cbegin(rhs.image_data),
                   std::ranges::begin(image_data), std::divides<>{});
            return *this;
        }

        friend bool operator==(Image<ElementT> const&, Image<ElementT> const&) = default;

        friend bool operator!=(Image<ElementT> const&, Image<ElementT> const&) = default;

        friend Image<ElementT> operator+(Image<ElementT> input1, const Image<ElementT>& input2)
        {
            return input1 += input2;
        }

        friend Image<ElementT> operator-(Image<ElementT> input1, const Image<ElementT>& input2)
        {
            return input1 -= input2;
        }

        friend Image<ElementT> operator*(Image<ElementT> input1, ElementT input2)
        {
            return multiplies(input1, input2);
        }

        friend Image<ElementT> operator*(ElementT input1, Image<ElementT> input2)
        {
            return multiplies(input2, input1);
        }
        
#ifdef USE_BOOST_SERIALIZATION

        void Save(std::string filename)
        {
            const std::string filename_with_extension = filename + ".dat";
            //  Reference: https://stackoverflow.com/questions/523872/how-do-you-serialize-an-object-in-c
            std::ofstream ofs(filename_with_extension, std::ios::binary);
            boost::archive::binary_oarchive ArchiveOut(ofs);
            //  write class instance to archive
            ArchiveOut << *this;
            //  archive and stream closed when destructors are called
            ofs.close();
        }
        
#endif
    private:
        std::vector<std::size_t> size;
        std::vector<ElementT> image_data;

        template<typename... Args>
        void checkBoundary(const Args... indexInput) const
        {
            constexpr std::size_t n = sizeof...(Args);
            if(n != size.size())
            {
                throw std::runtime_error("Dimensionality mismatched!");
            }
            std::size_t parameter_pack_index = 0;
            auto function = [&](auto index) {
                if (index >= size[parameter_pack_index])
                    throw std::out_of_range("Given index out of range!");
                parameter_pack_index = parameter_pack_index + 1;
            };

            (function(indexInput), ...);
        }

        //  checkBoundaryTuple template function implementation
        template<class TupleT>
        requires(TinyDIP::is_tuple<TupleT>::value)
        constexpr bool checkBoundaryTuple(const TupleT location)
        {
            constexpr std::size_t n = std::tuple_size<TupleT>{};
            if(n != size.size())
            {
                throw std::runtime_error("Dimensionality mismatched!");
            }
            std::size_t parameter_pack_index = 0;
            auto function = [&](auto index) {
                if (std::cmp_greater_equal(index, size[parameter_pack_index]))
                    return false;
                parameter_pack_index = parameter_pack_index + 1;
                return true;
            };
            return std::apply([&](auto&&... args) { return ((function(args))&& ...);}, location);
        }

        //  tuple_location_to_index template function implementation
        template<class TupleT>
        requires(TinyDIP::is_tuple<TupleT>::value)
        constexpr std::size_t tuple_location_to_index(TupleT location)
        {
            std::size_t i = 0;
            std::size_t stride = 1;
            std::size_t position = 0;
            auto update_position = [&](auto index) {
                    position += index * stride;
                    stride *= size[i++];
                };
            std::apply([&](auto&&... args) {((update_position(args)), ...);}, location);
            return position;
        }

    };

    template <typename T>
    struct is_Image : std::false_type {};

    template <typename T>
    struct is_Image<Image<T>> : std::true_type {};
}

timer class implementation:

namespace TinyDIP
{
    class Timer
    {
    private:
        std::chrono::system_clock::time_point start, end;
        std::chrono::duration<double> elapsed_seconds;
        std::time_t end_time;
    public:
        Timer()
        {
            start = std::chrono::system_clock::now();
        }
    
        ~Timer()
        {
            end = std::chrono::system_clock::now();
            elapsed_seconds = end - start;
            end_time = std::chrono::system_clock::to_time_t(end);
            if (elapsed_seconds.count() != 1)
            {
                std::print(std::cout, "Computation finished at {} elapsed time: {} seconds.\n", std::ctime(&end_time), elapsed_seconds.count());
            }
            else
            {
                std::print(std::cout, "Computation finished at {} elapsed time: {} second.\n", std::ctime(&end_time), elapsed_seconds.count());
            }
        }
    };
}

The usage of randi template function:

/* Developed by Jimmy Hu */

#include <chrono>
#include "../base_types.h"
#include "../basic_functions.h"
#include "../image.h"
#include "../image_operations.h"
#include "../timer.h"

void randiFunctionTest(
    const std::size_t sizex = 3,
    const std::size_t sizey = 2)
{
    //  Zero-dimensional result, an `Image<ElementT>` is returned
    auto randi_output1 = TinyDIP::randi(10);
    std::cout << "Zero-dimensional result: " << randi_output1 << '\n';

    //  Zero-dimensional result with specified range
    auto randi_output2 = TinyDIP::randi(std::make_pair(10, 100));
    std::cout << "Zero-dimensional result with specified range: " << randi_output2 << '\n';

    //  One-dimensional result
    auto randi_output3 = TinyDIP::randi(10, sizex);
    std::cout << "One-dimensional result: \n";
    randi_output3.print();

    //  sizex-by-sizey image of pseudorandom integers
    auto randi_output4 = TinyDIP::randi(10, sizex, sizey);
    std::cout << "sizex-by-sizey image of pseudorandom integers: \n";
    randi_output4.print();

    //  One-dimensional result with specified range
    auto randi_output5 = TinyDIP::randi(std::make_pair(10, 100), sizex);
    std::cout << "One-dimensional result with specified range: \n";
    randi_output5.print();

    //  sizex-by-sizey image of pseudorandom integers with specified range
    auto randi_output6 = TinyDIP::randi(std::make_pair(10, 100), sizex, sizey);
    std::cout << "sizex-by-sizey image of pseudorandom integers with specified range: \n";
    randi_output6.print();

    return;
}

int main()
{
    TinyDIP::Timer timer1;
    randiFunctionTest();
    return EXIT_SUCCESS;
}

The output of the test code above:

Zero-dimensional result: 1

Zero-dimensional result with specified range: 79

One-dimensional result:
6       4       2
sizex-by-sizey image of pseudorandom integers:
3       2       9
4       4       4

One-dimensional result with specified range:
10      85      68
sizex-by-sizey image of pseudorandom integers with specified range:
24      39      50
25      68      65

Computation finished at Thu Mar 13 19:36:48 2025
 elapsed time: 0.0500522 seconds.

TinyDIP on GitHub

All suggestions are welcome.

The summary information:

• Which question it is a follow-up to?

  An Updated Multi-dimensional Image Data Structure with Variadic Template Functions in C++,

  rand Template Function Implementation for Image in C++,

  randi Template Function Implementation for Image in C++ and

  randi Template Function Implementation for Image in C++ (Rev.2)

• What changes has been made in the code since last question?

  The randi template function implementation and some of Image class constructors have been updated in this post.

• Why a new review is being asked for?

  Please review the current version implementation of randi template function and its tests.

Answer 1

Unnecessary check for sizeof...(Sizes) == 0

There is no need for the check for an empty parameter pack in randi(); generate() will already handle this case for you. So:

template<std::integral ElementT = int, typename Urbg, std::same_as<std::size_t>... Sizes>
requires std::uniform_random_bit_generator<std::remove_reference_t<Urbg>>
constexpr static auto randi(Urbg&& urbg, std::pair<ElementT, ElementT> min_and_max, Sizes... sizes)
{
    auto dist = std::uniform_int_distribution<ElementT>{ min_and_max.first, min_and_max.second };
    return generate([&dist, &urbg]() { return dist(urbg); }, sizes...);
}

This is because the overload of generate() that takes a parameter will construct a std::array of zero elements, which is valid, and then the other overload will try to std::reduce that, which is also valid, and the result will be 1, as that is the initial value you correctly provided to std::reduce(). Finally, creating an Image object with a vector as the first argument and nothing else will use the constructor that takes a parameter pack of sizes, but that one also handles an empty parameter pack correctly.

Unnecessary overload that just takes a max

The last overload of randi() is not necessary. Again, empty parameter packs are already handled correctly everywhere, so if you remove that one, a function call with just one integer parameter will use the overload that takes an integer and a parameter pack of sizes.

If this overload was necessary, then it would also have been necessary to create an overload that takes just a min_and_max pair.

Avoidable copies and/or moves of elements

While you did implement a constructor of Image that uses std::from_range_t, this still is not optimal. First, even though you are constructing a std::vector from another std::vector, when using std::from_range_t, it will not actually just move the pointer to the underlying data, instead it will move elements individually. And since ElementT is probably just a few integers or floats, that means it just ends up copying everything.

Another issue is that you create element_vector(count), which unncessarily initializes all the elements right before you overwrite them.

Barring a hypothetical std::views::generate() (although Range-v3 has one), the most optimal way to handle this is to push back elements into an initially empty vector, and then to pass this to a constructor for Image that explicitly takes a std::vector by r-value:

template <typename ElementT>
class Image
{
    …
    template<std::ranges::input_range Sizes>
    requires(std::same_as<std::ranges::range_value_t<Sizes>, std::size_t>)
    Image(std::vector<ElementT>&& input, const Sizes& sizes)
    {
        if (input.size() != std::fold_left(sizes, 1, std::multiplies{})) {
            throw std::runtime_error("Image data input and the given size are mismatched!");
        }
        size.assign_range(sizes)
        image_data = std::move(input);
    }
    …
};

template<std::ranges::input_range Sizes, typename F>
requires((std::same_as<std::ranges::range_value_t<Sizes>, std::size_t>) and
         (std::invocable<F&>))
constexpr static auto generate(F gen, const Sizes& sizes)
{
    using ElementT = std::invoke_result_t<F>;
    auto count = std::fold_left(sizes, 1, std::multiplies{});
    std::vector<ElementT> element_vector;
    element_vector.reserve(count);
    std::ranges::generate_n(std::back_inserter(element_vector), count, gen);
    return Image(element_vector, sizes);
}