2
\$\begingroup\$

This is a follow-up question for 3D Inverse Discrete Cosine Transformation Implementation in C++. After checking G. Sliepen's answer, I am trying to update the part of width and height checking of Image class. Instead using macros, there are several template functions is_width_same, is_height_same, is_size_same, assert_width_same, assert_height_same, assert_size_same and check_size_same proposed in this post.

The experimental implementation

  • is_width_same template functions implementation:

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

  • is_height_same template functions implementation:

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

  • is_size_same template functions implementation:

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

  • assert_width_same template functions implementation: wrap is_width_same function with assert.

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

  • assert_height_same template function implementation: wrap is_height_same function with assert.

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

  • assert_size_same template function implementation:

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

  • check_size_same template function implementation:

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

  • the updated version Image class:

    • operator<< overloading updated.

    • checkBoundary function implementation updated.

    • I am trying to add a constructor with rvalue reference Image(std::vector<ElementT>&& input, std::size_t newWidth, std::size_t newHeight). Please also take a look about this part.

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

    Image(const std::size_t width, const std::size_t height):
        width(width),
        height(height),
        image_data(width * height) { }

    Image(const std::size_t width, const std::size_t height, const ElementT initVal):
        width(width),
        height(height),
        image_data(width * height, initVal) {}

    Image(const std::vector<ElementT>& input, std::size_t newWidth, std::size_t newHeight):
        width(newWidth),
        height(newHeight)
    {
        if (input.size() != newWidth * newHeight)
        {
            throw std::runtime_error("Image data input and the given size are mismatched!");
        }
        image_data = input;
    }

    Image(std::vector<ElementT>&& input, std::size_t newWidth, std::size_t newHeight):
        width(newWidth),
        height(newHeight)
    {
        if (input.size() != newWidth * newHeight)
        {
            throw std::runtime_error("Image data input and the given size are mismatched!");
        }
        image_data = std::move(input);
    }

    constexpr ElementT& at(const unsigned int x, const unsigned int y)
    { 
        checkBoundary(x, y);
        return image_data[y * width + x];
    }

    constexpr ElementT const& at(const unsigned int x, const unsigned int y) const
    {
        checkBoundary(x, y);
        return image_data[y * width + x];
    }

    constexpr std::size_t getWidth() const
    {
        return width;
    }

    constexpr std::size_t getHeight() const noexcept
    {
        return height;
    }

    constexpr auto getSize() noexcept
    {
        return std::make_tuple(width, height);
    }

    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
    {
        for (std::size_t y = 0; y < height; ++y)
        {
            for (std::size_t x = 0; x < width; ++x)
            {
                //  Ref: https://isocpp.org/wiki/faq/input-output#print-char-or-ptr-as-number
                os << +at(x, y) << separator;
            }
            os << "\n";
        }
        os << "\n";
        return;
    }

    //  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 < height; ++y)
        {
            for (std::size_t x = 0; x < width; ++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;
    }

    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)
    {
        assert(rhs.width == this->width);
        assert(rhs.height == this->height);
        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)
    {
        assert(rhs.width == this->width);
        assert(rhs.height == this->height);
        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)
    {
        assert(rhs.width == this->width);
        assert(rhs.height == this->height);
        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)
    {
        assert(rhs.width == this->width);
        assert(rhs.height == this->height);
        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;
    }

    Image<ElementT>& operator=(Image<ElementT> const& input) = default;  //  Copy Assign

    Image<ElementT>& operator=(Image<ElementT>&& other) = default;       //  Move Assign

    Image(const Image<ElementT> &input) = default;                       //  Copy Constructor

    Image(Image<ElementT> &&input) = default;                            //  Move Constructor

private:
    std::size_t width;
    std::size_t height;
    std::vector<ElementT> image_data;

    void checkBoundary(const size_t x, const size_t y) const
    {
        if (x >= width)
            throw std::out_of_range("Given x out of range!");
        if (y >= height)
            throw std::out_of_range("Given y out of range!");
    }
};

Full Testing Code

#include <algorithm>
#include <cassert>
#include <chrono>
#include <cmath>
#include <concepts>
#include <cstdint>
#include <exception>
#include <fstream>
#include <functional>
#include <iostream>
#include <iterator>
#include <numbers>
#include <numeric>
#include <ranges>
#include <string>
#include <type_traits>
#include <utility>
#include <vector>

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

using GrayScale = std::uint8_t;

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

        Image(const std::size_t width, const std::size_t height):
            width(width),
            height(height),
            image_data(width * height) { }

        Image(const std::size_t width, const std::size_t height, const ElementT initVal):
            width(width),
            height(height),
            image_data(width * height, initVal) {}

        Image(const std::vector<ElementT>& input, std::size_t newWidth, std::size_t newHeight):
            width(newWidth),
            height(newHeight)
        {
            if (input.size() != newWidth * newHeight)
            {
                throw std::runtime_error("Image data input and the given size are mismatched!");
            }
            image_data = input;
        }

        Image(std::vector<ElementT>&& input, std::size_t newWidth, std::size_t newHeight):
            width(newWidth),
            height(newHeight)
        {
            if (input.size() != newWidth * newHeight)
            {
                throw std::runtime_error("Image data input and the given size are mismatched!");
            }
            image_data = std::move(input);
        }

        constexpr ElementT& at(const unsigned int x, const unsigned int y)
        { 
            checkBoundary(x, y);
            return image_data[y * width + x];
        }

        constexpr ElementT const& at(const unsigned int x, const unsigned int y) const
        {
            checkBoundary(x, y);
            return image_data[y * width + x];
        }

        constexpr std::size_t getWidth() const
        {
            return width;
        }

        constexpr std::size_t getHeight() const noexcept
        {
            return height;
        }

        constexpr auto getSize() noexcept
        {
            return std::make_tuple(width, height);
        }

        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
        {
            for (std::size_t y = 0; y < height; ++y)
            {
                for (std::size_t x = 0; x < width; ++x)
                {
                    //  Ref: https://isocpp.org/wiki/faq/input-output#print-char-or-ptr-as-number
                    os << +at(x, y) << separator;
                }
                os << "\n";
            }
            os << "\n";
            return;
        }

        //  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 < height; ++y)
            {
                for (std::size_t x = 0; x < width; ++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;
        }

        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)
        {
            assert(rhs.width == this->width);
            assert(rhs.height == this->height);
            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)
        {
            assert(rhs.width == this->width);
            assert(rhs.height == this->height);
            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)
        {
            assert(rhs.width == this->width);
            assert(rhs.height == this->height);
            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)
        {
            assert(rhs.width == this->width);
            assert(rhs.height == this->height);
            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;
        }

        Image<ElementT>& operator=(Image<ElementT> const& input) = default;  //  Copy Assign

        Image<ElementT>& operator=(Image<ElementT>&& other) = default;       //  Move Assign

        Image(const Image<ElementT> &input) = default;                       //  Copy Constructor

        Image(Image<ElementT> &&input) = default;                            //  Move Constructor
        
    private:
        std::size_t width;
        std::size_t height;
        std::vector<ElementT> image_data;

        void checkBoundary(const size_t x, const size_t y) const
        {
            if (x >= width)
                throw std::out_of_range("Given x out of range!");
            if (y >= height)
                throw std::out_of_range("Given y out of range!");
        }
    };

    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_size_same(const Image<ElementT>& x, const Image<ElementT>& y)
    {
        if (!is_width_same(x, y))
            throw std::runtime_error("Width mismatched!");
        if (!is_height_same(x, y))
            throw std::runtime_error("Height mismatched!");
    }
}

void checkSizeTest(const std::size_t xsize, const std::size_t ysize)
{
    auto image1 = TinyDIP::Image<std::uint8_t>(xsize, ysize);
    auto image2 = TinyDIP::Image<std::uint8_t>(xsize + 1, ysize);
    auto image3 = TinyDIP::Image<std::uint8_t>(xsize, ysize + 1);
    auto image4 = TinyDIP::Image<std::uint8_t>(xsize + 1, ysize + 1);
    check_size_same(image1, image1);
    return;
}

int main()
{
    auto start = std::chrono::system_clock::now();
    checkSizeTest(8, 8);
    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;
}

A Godbolt link is here.

TinyDIP on GitHub

All suggestions are welcome.

The summary information:

  • Which question it is a follow-up to?

    3D Inverse Discrete Cosine Transformation Implementation in C++

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

    There are several template functions is_width_same, is_height_same, is_size_same, assert_width_same, assert_height_same, assert_size_same and check_size_same proposed in this post.

  • Why a new review is being asked for?

    If there is any possible improvement, please let me know.

\$\endgroup\$

2 Answers 2

Reset to default
2
\$\begingroup\$

Make it work for any number of images

I see you have overloads for comparing two and for comparing three images. But that immediately makes me think: why not compare four? Or more? You can make a variadic function that uses a fold expression to check the dimensions of an arbitrary number of images with each other:

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

As a bonus, this will also allow you to check that the dimensions of two images using a different value type are the same. If you really don't want that, you can add a requires clause to force them to all be the same:

requires (std::same_as<T, Ts> && ...)

Consider removing the assert_*_same() helpers

You are not saving much typing with these helper functions, compare:

assert(is_width_same(x, y));
assert_width_same(x, y);

It saves only 4 characters, but the drawback is that when the assert triggers, the second one will show a line number inside assert_width_same() instead of the line number of the call site.

It is different for check_size_same(); at least on Linux, if an unhandled exception is thrown, it prints the what() but no line number, so nothing is lost by putting the throw statements in a function.

\$\endgroup\$
2
\$\begingroup\$

One of the three tests here is superfluous:

is_width_same(x, y) && is_width_same(y, z) && is_width_same(x, z);

Once we know that x==y and y==z, it is already established that x==z, there is no need to add that test explicitly.

With this template:

template<typename ElementT>
constexpr bool is_width_same(const Image<ElementT>& x, const Image<ElementT>& y)

we can only compare images with the same pixel type. What if you want to implement addition between two images of different type? What if you want to implement indexing an arbitrary image with a binary image? In these cases, it would be more useful to implement

template<typename ElementTx, ElementTy>
constexpr bool is_width_same(const Image<ElementTx>& x, const Image<ElementTy>& y)

I see lots of functions like this:

Image<ElementT>& operator+=(const Image<ElementT>& rhs)
{
    assert(rhs.width == this->width);
    assert(rhs.height == this->height);
    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;
}

Why not use your new check_size_same function?

\$\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.