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: wrapis_width_same
function withassert
.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: wrapis_height_same
function withassert
.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;
}
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
andcheck_size_same
proposed in this post.Why a new review is being asked for?
If there is any possible improvement, please let me know.
2 Answers 2
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.
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?