1
\$\begingroup\$

This is a follow-up question for Multi-dimensional Image Data Structure with Variadic Template Functions in C++. Considering the suggestion from G. Sliepen:

Make everything work for more than 5 dimensions

I can see why you stopped at 5 dimensions if you still have to manually write the code for each number of dimensions separately. Just spend a little bit of time to figure out how to write this in a generic way, then you'll actually have to do less typing and make the code work for any number of dimensions.

Whenever you have to loop over a parameter pack, you can always do this:

auto function = [&](auto index) {
    ...
};

(function(indexInput), ...);

Now function() will be called for every element in indexInput in succession. In that lambda you can do anything you want, including incrementing a loop counter. So use that to build up the index you need to read from image_data. This is left as an excercise for the reader.

I am trying to do the excercise of looping over parameter packs with the given structure.

The experimental implementation

  • Image Class Implementation

    //  image.h
    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::ranges::input_range Range,
                    std::same_as<std::size_t>... Sizes>
            Image(const Range& input, Sizes... sizes):
                size{sizes...}, image_data(begin(input), end(input))
            {
                if (image_data.size() != (1 * ... * sizes)) {
                    throw std::runtime_error("Image data input and the given size are mismatched!");
                }
            }
    
            Image(std::vector<ElementT>&& input, std::size_t newWidth, std::size_t newHeight)
            {
                size.reserve(2);
                size.emplace_back(newWidth);
                size.emplace_back(newHeight);
                if (input.size() != newWidth * newHeight)
                {
                    throw std::runtime_error("Image data input and the given size are mismatched!");
                }
                image_data = std::move(input);              //  Reference: https://stackoverflow.com/a/51706522/6667035
            }
    
            Image(const std::vector<std::vector<ElementT>>& input)
            {
                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(input), std::ranges::end(input));    //  flatten
                }
                return;
            }
    
            template<typename... Args>
            constexpr ElementT& at(const Args... indexInput)
            {
                checkBoundary(indexInput...);
                constexpr std::size_t n = sizeof...(Args);
                if(n != size.size())
                {
                    throw std::runtime_error("Dimensionality mismatched!");
                }
                std::size_t parameter_pack_index = 0;
                std::size_t image_data_index = 0;
                auto function = [&](auto index) {
                    std::size_t m = 1;
                    for(std::size_t i = 0; i < parameter_pack_index; ++i)
                    {
                        m*=size[i];
                    }
                    image_data_index+=(index * m);
                    parameter_pack_index = parameter_pack_index + 1;
                };
                (function(indexInput), ...);
                return image_data[image_data_index];
            }
    
            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 parameter_pack_index = 0;
                std::size_t image_data_index = 0;
                auto function = [&](auto index) {
                    std::size_t m = 1;
                    for(std::size_t i = 0; i < parameter_pack_index; ++i)
                    {
                        m*=size[i];
                    }
                    image_data_index+=(index * m);
                    parameter_pack_index = parameter_pack_index + 1;
                };
                (function(indexInput), ...);
                return image_data[image_data_index];
            }
    
            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];
            }
    
            constexpr auto getSize() noexcept
            {
                return size;
            }
    
            std::vector<ElementT> const& getImageData() const noexcept { return image_data; }      //  expose the internal data
    
            void print(std::string separator = "\t", std::ostream& os = std::cout) const
            {
                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";
                }
            }
    
            //  Enable this function if ElementT = RGB
            void print(std::string separator = "\t", std::ostream& os = std::cout) const
            requires(std::same_as<ElementT, RGB>)
            {
                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;
            }
    
            void setAllValue(const ElementT input)
            {
                std::fill(image_data.begin(), image_data.end(), input);
            }
    
            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), ...);
            }
        };
    
        template<typename T, typename ElementT>
        concept is_Image = std::is_same_v<T, Image<ElementT>>;
    }
    

Full Testing Code

The full testing code:

//  An Updated Multi-dimensional Image Data Structure with Variadic Template Functions in C++
//  Developed by Jimmy Hu

#include <algorithm>
#include <cassert>      //  for assert
#include <chrono>       //  for std::chrono::system_clock::now
#include <cmath>        //  for std::exp
#include <concepts>
#include <execution>    //  for std::is_execution_policy_v
#include <iostream>     //  for std::cout
#include <vector>

struct RGB
{
    std::uint8_t channels[3];
};

using GrayScale = std::uint8_t;

namespace TinyDIP
{
    //  recursive_depth function implementation
    template<typename T>
    constexpr std::size_t recursive_depth()
    {
        return std::size_t{0};
    }

    template<std::ranges::input_range Range>
    constexpr std::size_t recursive_depth()
    {
        return recursive_depth<std::ranges::range_value_t<Range>>() + std::size_t{1};
    }

    template<std::size_t index = 1, typename Arg, typename... Args>
    constexpr static auto& get_from_variadic_template(const Arg& first, const Args&... inputs)
    {
        if constexpr (index > 1)
            return get_from_variadic_template<index - 1>(inputs...);
        else
            return first;
    }

    template<typename... Args>
    constexpr static auto& first_of(Args&... inputs) {
        return get_from_variadic_template<1>(inputs...);
    }

    template<std::size_t, typename, typename...>
    struct get_from_variadic_template_struct { };

    template<typename T1, typename... Ts>
    struct get_from_variadic_template_struct<1, T1, Ts...>
    {
        using type = T1;
    };

    template<std::size_t index, typename T1, typename... Ts>
    requires ( requires { typename get_from_variadic_template_struct<index - 1, Ts...>::type; })
    struct get_from_variadic_template_struct<index, T1, Ts...>
    {
        using type = typename get_from_variadic_template_struct<index - 1, Ts...>::type;
    };

    template<std::size_t index, typename... Ts>
    using get_from_variadic_template_t = typename get_from_variadic_template_struct<index, Ts...>::type;
}

//  image.h
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::ranges::input_range Range,
                std::same_as<std::size_t>... Sizes>
        Image(const Range& input, Sizes... sizes):
            size{sizes...}, image_data(begin(input), end(input))
        {
            if (image_data.size() != (1 * ... * sizes)) {
                throw std::runtime_error("Image data input and the given size are mismatched!");
            }
        }

        Image(std::vector<ElementT>&& input, std::size_t newWidth, std::size_t newHeight)
        {
            size.reserve(2);
            size.emplace_back(newWidth);
            size.emplace_back(newHeight);
            if (input.size() != newWidth * newHeight)
            {
                throw std::runtime_error("Image data input and the given size are mismatched!");
            }
            image_data = std::move(input);              //  Reference: https://stackoverflow.com/a/51706522/6667035
        }

        Image(const std::vector<std::vector<ElementT>>& input)
        {
            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(input), std::ranges::end(input));    //  flatten
            }
            return;
        }

        template<typename... Args>
        constexpr ElementT& at(const Args... indexInput)
        {
            checkBoundary(indexInput...);
            constexpr std::size_t n = sizeof...(Args);
            if(n != size.size())
            {
                throw std::runtime_error("Dimensionality mismatched!");
            }
            std::size_t parameter_pack_index = 0;
            std::size_t image_data_index = 0;
            auto function = [&](auto index) {
                std::size_t m = 1;
                for(std::size_t i = 0; i < parameter_pack_index; ++i)
                {
                    m*=size[i];
                }
                image_data_index+=(index * m);
                parameter_pack_index = parameter_pack_index + 1;
            };
            (function(indexInput), ...);
            return image_data[image_data_index];
        }

        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 parameter_pack_index = 0;
            std::size_t image_data_index = 0;
            auto function = [&](auto index) {
                std::size_t m = 1;
                for(std::size_t i = 0; i < parameter_pack_index; ++i)
                {
                    m*=size[i];
                }
                image_data_index+=(index * m);
                parameter_pack_index = parameter_pack_index + 1;
            };
            (function(indexInput), ...);
            return image_data[image_data_index];
        }
  
        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];
        }

        constexpr auto getSize() noexcept
        {
            return size;
        }

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

        void print(std::string separator = "\t", std::ostream& os = std::cout) const
        {
            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";
            }
        }

        //  Enable this function if ElementT = RGB
        void print(std::string separator = "\t", std::ostream& os = std::cout) const
        requires(std::same_as<ElementT, RGB>)
        {
            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;
        }

        void setAllValue(const ElementT input)
        {
            std::fill(image_data.begin(), image_data.end(), input);
        }

        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), ...);
        }
    };

    template<typename T, typename ElementT>
    concept is_Image = std::is_same_v<T, Image<ElementT>>;
}

namespace TinyDIP
{
    template<typename ElementT>
    constexpr bool is_width_same(const Image<ElementT>& x, const Image<ElementT>& y)
    {
        return x.getWidth() == y.getWidth();
    }

    template<typename ElementT>
    constexpr bool is_width_same(const Image<ElementT>& x, const Image<ElementT>& y, const Image<ElementT>& z)
    {
        return is_width_same(x, y) && is_width_same(y, z) && is_width_same(x, z);
    }

    template<typename ElementT>
    constexpr bool is_height_same(const Image<ElementT>& x, const Image<ElementT>& y)
    {
        return x.getHeight() == y.getHeight();
    }

    template<typename ElementT>
    constexpr bool is_height_same(const Image<ElementT>& x, const Image<ElementT>& y, const Image<ElementT>& z)
    {
        return is_height_same(x, y) && is_height_same(y, z) && is_height_same(x, z);
    }
    
    template<typename ElementT>
    constexpr bool is_size_same(const Image<ElementT>& x, const Image<ElementT>& y)
    {
        return is_width_same(x, y) && is_height_same(x, y);
    }

    template<typename ElementT>
    constexpr bool is_size_same(const Image<ElementT>& x, const Image<ElementT>& y, const Image<ElementT>& z)
    {
        return is_size_same(x, y) && is_size_same(y, z) && is_size_same(x, z);
    }

    template<typename ElementT>
    constexpr void assert_width_same(const Image<ElementT>& x, const Image<ElementT>& y)
    {
        assert(is_width_same(x, y));
    }

    template<typename ElementT>
    constexpr void assert_width_same(const Image<ElementT>& x, const Image<ElementT>& y, const Image<ElementT>& z)
    {
        assert(is_width_same(x, y, z));
    }

    template<typename ElementT>
    constexpr void assert_height_same(const Image<ElementT>& x, const Image<ElementT>& y)
    {
        assert(is_height_same(x, y));
    }

    template<typename ElementT>
    constexpr void assert_height_same(const Image<ElementT>& x, const Image<ElementT>& y, const Image<ElementT>& z)
    {
        assert(is_height_same(x, y, z));
    }

    template<typename ElementT>
    constexpr void assert_size_same(const Image<ElementT>& x, const Image<ElementT>& y)
    {
        assert_width_same(x, y);
        assert_height_same(x, y);
    }

    template<typename ElementT>
    constexpr void assert_size_same(const Image<ElementT>& x, const Image<ElementT>& y, const Image<ElementT>& z)
    {
        assert_size_same(x, y);
        assert_size_same(y, z);
        assert_size_same(x, z);
    }

    template<typename ElementT>
    constexpr void check_width_same(const Image<ElementT>& x, const Image<ElementT>& y)
    {
        if (!is_width_same(x, y))
            throw std::runtime_error("Width mismatched!");
    }

    template<typename ElementT>
    constexpr void check_height_same(const Image<ElementT>& x, const Image<ElementT>& y)
    {
        if (!is_height_same(x, y))
            throw std::runtime_error("Height mismatched!");
    }

    template<typename ElementT>
    constexpr void check_size_same(const Image<ElementT>& x, const Image<ElementT>& y)
    {
        check_width_same(x, y);
        check_height_same(x, y);
    }
}

template<typename ElementT>
void multidimensionalImageTest(const size_t size = 5)
{
    std::cout << "Test with 1D image:\n";
    auto image1d = TinyDIP::Image<ElementT>(size);
    image1d.at(0) = 3;
    image1d.print();
    std::cout << "Test with 2D image:\n";
    auto image2d = TinyDIP::Image<ElementT>(size, size);
    image2d.setAllValue(1);
    image2d.at(0, 1) = 3;
    image2d.print();
    std::cout << "Test with 3D image:\n";
    auto image3d = TinyDIP::Image<double>(size, size, size);
    image3d.setAllValue(0);
    image3d.at(3, 1, 0) = 4;
    image3d.print();
    return;
}

int main()
{
    auto start = std::chrono::system_clock::now();
    multidimensionalImageTest<double>();
    auto end = std::chrono::system_clock::now();
    std::chrono::duration<double> elapsed_seconds = end - start;
    std::time_t end_time = std::chrono::system_clock::to_time_t(end);
    std::cout << "Computation finished at " << std::ctime(&end_time) << "elapsed time: " << elapsed_seconds.count() << '\n';
    return 0;
}

The output of the test code above:

Test with 1D image:
3   0   0   0   0   
Test with 2D image:
1   1   1   1   1   
3   1   1   1   1   
1   1   1   1   1   
1   1   1   1   1   
1   1   1   1   1   

Test with 3D image:
0   0   0   0   0   
0   0   0   4   0   
0   0   0   0   0   
0   0   0   0   0   
0   0   0   0   0   

0   0   0   0   0   
0   0   0   0   0   
0   0   0   0   0   
0   0   0   0   0   
0   0   0   0   0   

0   0   0   0   0   
0   0   0   0   0   
0   0   0   0   0   
0   0   0   0   0   
0   0   0   0   0   

0   0   0   0   0   
0   0   0   0   0   
0   0   0   0   0   
0   0   0   0   0   
0   0   0   0   0   

0   0   0   0   0   
0   0   0   0   0   
0   0   0   0   0   
0   0   0   0   0   
0   0   0   0   0   


Computation finished at Tue Jan  2 03:25:03 2024
elapsed time: 0.00127554

Godbolt link is here.

All suggestions are welcome.

The summary information:

  • Which question it is a follow-up to?

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

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

    I am trying to do the excercise of looping over parameter packs with the given structure.

  • Why a new review is being asked for?

    Please review the updated Image template class implementation and all suggestions are welcome.

\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

Apply the techniques you have learned to all of the code

In the past reviews of your questions we have repeatedly stressed that you should simplify the code, reduce code duplication and make it more generic. Those things also usually go hand in hand. However, you are not applying these lessons consistently to all of your code. Next time, a good excercise will be to review your code yourself first, and think about what you might do to improve it. This time, I will just have to repeat myself.

Please don't take this admonishment personally though, you are not the only person doing this, and I probably am guilty of this myself. But I do want to make you aware of this issue so you can improve.

Make everything generic

Here is a good excercise: make everything you see generic where possible. That constructor that takes a std::vector<ElementT> and a bunch of sizes? Make it generic so it handles any number of dimensions. The constructor that takes a std::vector<std::vector<ElementT>>? Make it so it handles an arbitrary number of nested vectors. Why fixate on std::vectors? Make things work for any kind of range. Storing image_data in a vector? Why not make Image so generic the storage type is also just a template parameter, and make it handle custom allocators as well? Those print() functions? Generic! Getting the size of a given dimension? Generic!

Avoid code duplication

Apart from the code you can save by making things more generic, you also have the issue of at(), where you need both a const and non-const version. But you can avoid a lot of duplication there. You can split out the index calculation into a separate function, which will get rid of most of the code duplication. However, you can also have one version call the other and apply a bit of legal const_cast<>()ing, see this StackOverflow question.

Simplify the code

The code to calculate the index is still looking a bit more complex than necessary to me. Every time function() is called, you have a for-loop to calculate m from scratch. However, if you move m out of the lambda's body you don't need that loop.

You also don't need the check for sizeof...(Args), as that is already done by checkBoundary().

Here is a simplified version:

template<typename... Args>
constexpr ElementT const& at(const Args... indexInput) const
{
    checkBoundary(indexInput...);

    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];
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.