2
\$\begingroup\$

This is a follow-up question for Two dimensional gaussian image generator in C++. Thanks for Cris Luengo's answer and JDługosz's answer. I am attempting to update the implementation of Image template class, including its constructor definitions and the check of size info. A private member function checkBoundary is added here for boundary checking.

The experimental implementation

  • namespace: TinyDIP

  • The tests for the operators of image template class:

    void imageElementwiseAddTest()
    {
        std::cout << "imageElementwiseAddTest\n";
        auto test = TinyDIP::Image<int>(10, 10, 1);
        test+=test;
        test.print();
    
        auto test2 = TinyDIP::Image<int>(11, 11, 1);
        test+=test2;            //  different size case for failure detection
        test.print();
    
    }
    
    void imageElementwiseMinusTest()
    {
        std::cout << "imageElementwiseMinusTest\n";
        auto test = TinyDIP::Image<int>(10, 10, 1);
        test-=test;
        test.print();
    
        auto test2 = TinyDIP::Image<int>(11, 11, 1);
        test-=test2;            //  different size case for failure detection
        test.print();
    }
    
    
    void imageElementwiseMultipliesTest()
    {
        std::cout << "imageElementwiseMultipliesTest\n";
        auto test = TinyDIP::Image<int>(10, 10, 1);
        test*=test;
        test.print();
    
        auto test2 = TinyDIP::Image<int>(11, 11, 1);
        test*=test2;            //  different size case for failure detection
        test.print();
    }
    
    void imageElementwiseDividesTest()
    {
        std::cout << "imageElementwiseDividesTest\n";
        auto test = TinyDIP::Image<int>(10, 10, 1);
        test/=test;
        test.print();
    
        auto test2 = TinyDIP::Image<int>(11, 11, 1);
        test/=test2;            //  different size case for failure detection
        test.print();
    }
    
  • Image template class implementation (image.h):

    /* Developed by Jimmy Hu */
    
    #ifndef Image_H
    #define Image_H
    
    #include <algorithm>
    #include <array>
    #include <cassert>
    #include <chrono>
    #include <complex>
    #include <concepts>
    #include <functional>
    #include <iostream>
    #include <iterator>
    #include <list>
    #include <numeric>
    #include <string>
    #include <type_traits>
    #include <variant>
    #include <vector>
    #include "image_operations.h"
    
    namespace TinyDIP
    {
        template <typename ElementT>
        class Image
        {
        public:
            Image() = default;
    
            Image(const size_t width, const size_t height):
                width(width),
                height(height),
                image_data(width * height) { }
    
            Image(const int width, const int height, const ElementT initVal):
                width(width),
                height(height),
                image_data(width * height, initVal) {}
    
            Image(const std::vector<ElementT>& input, size_t newWidth, size_t newHeight):
                width(newWidth),
                height(newHeight)
            {
                assert(input.size() == newWidth * newHeight);
                this->image_data = input;   //  Deep copy
            }
    
            Image(const std::vector<std::vector<ElementT>>& input)
            {
                this->height = input.size();
                this->width = input[0].size();
    
                for (auto& rows : input)
                {
                    this->image_data.insert(this->image_data.end(), std::begin(input), std::end(input));    //  flatten
                }
                return;
            }
    
            constexpr ElementT& at(const unsigned int x, const unsigned int y)
            { 
                checkBoundary(x, y);
                return this->image_data[y * width + x];
            }
    
            constexpr ElementT const& at(const unsigned int x, const unsigned int y) const
            {
                checkBoundary(x, y);
                return this->image_data[y * width + x];
            }
    
            constexpr size_t getWidth()
            {
                return this->width;
            }
    
            constexpr size_t getHeight()
            {
                return this->height;
            }
    
            std::vector<ElementT> const& getImageData() const { return this->image_data; }      //  expose the internal data
    
            void print()
            {
                for (size_t y = 0; y < this->height; ++y)
                {
                    for (size_t x = 0; x < this->width; ++x)
                    {
                        //  Ref: https://isocpp.org/wiki/faq/input-output#print-char-or-ptr-as-number
                        std::cout << +this->at(x, y) << "\t";
                    }
                    std::cout << "\n";
                }
                std::cout << "\n";
                return;
            }
    
            Image<ElementT>& operator+=(const Image<ElementT>& rhs)
            {
                assert(rhs.width == this->width);
                assert(rhs.height == this->height);
                std::transform(image_data.cbegin(), image_data.cend(), rhs.image_data.cbegin(),
                       image_data.begin(), std::plus<>{});
                return *this;
            }
    
            Image<ElementT>& operator-=(const Image<ElementT>& rhs)
            {
                assert(rhs.width == this->width);
                assert(rhs.height == this->height);
                std::transform(image_data.cbegin(), image_data.cend(), rhs.image_data.cbegin(),
                       image_data.begin(), std::minus<>{});
                return *this;
            }
    
            Image<ElementT>& operator*=(const Image<ElementT>& rhs)
            {
                assert(rhs.width == this->width);
                assert(rhs.height == this->height);
                std::transform(image_data.cbegin(), image_data.cend(), rhs.image_data.cbegin(),
                       image_data.begin(), std::multiplies<>{});
                return *this;
            }
    
            Image<ElementT>& operator/=(const Image<ElementT>& rhs)
            {
                assert(rhs.width == this->width);
                assert(rhs.height == this->height);
                std::transform(image_data.cbegin(), image_data.cend(), rhs.image_data.cbegin(),
                       image_data.begin(), std::divides<>{});
                return *this;
            }
    
            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:
            size_t width;
            size_t height;
            std::vector<ElementT> image_data;
    
            void checkBoundary(const size_t x, const size_t y)
            {
                assert(x < width);
                assert(y < height);
            }
        };
    }
    
    
    #endif
    
  • base_types.h: The base types

    /* Developed by Jimmy Hu */
    
    #ifndef BASE_H
    #define BASE_H
    
    #include <math.h>
    #include <stdio.h>
    #include <stdlib.h>
    #include <string>
    
    using BYTE = unsigned char;
    
    struct RGB
    {
        unsigned char channels[3];
    };
    
    using GrayScale = BYTE;
    
    struct HSV
    {
        double channels[3];    //  Range: 0 <= H < 360, 0 <= S <= 1, 0 <= V <= 255
    };
    
    #endif
    

The full testing code

/* Developed by Jimmy Hu */

#include "base_types.h"
#include "image.h"

void imageElementwiseAddTest();
void imageElementwiseMinusTest();
void imageElementwiseMultipliesTest();
void imageElementwiseDividesTest();

int main()
{
    imageElementwiseAddTest();
    imageElementwiseMinusTest();
    imageElementwiseMultipliesTest();
    imageElementwiseDividesTest();
    return 0;
}

void imageElementwiseAddTest()
{
    std::cout << "imageElementwiseAddTest\n";
    auto test = TinyDIP::Image<int>(10, 10, 1);
    test+=test;
    test.print();
}

void imageElementwiseMinusTest()
{
    std::cout << "imageElementwiseMinusTest\n";
    auto test = TinyDIP::Image<int>(10, 10, 1);
    test-=test;
    test.print();
}


void imageElementwiseMultipliesTest()
{
    std::cout << "imageElementwiseMultipliesTest\n";
    auto test = TinyDIP::Image<int>(10, 10, 1);
    test*=test;
    test.print();
}

void imageElementwiseDividesTest()
{
    std::cout << "imageElementwiseDividesTest\n";
    auto test = TinyDIP::Image<int>(10, 10, 1);
    test/=test;
    test.print();
}

All suggestions are welcome.

The summary information:

  • Which question it is a follow-up to?

    Two dimensional gaussian image generator in C++

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

    The check operation of size info in Image constructor and member functions has been added in this post.

  • Why a new review is being asked for?

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

\$\endgroup\$
1
\$\begingroup\$

Consider using a testing framework

I recommend you try out a (unit) testing framework like Google Test, cppunit, or one of the many others. Using these has some advantages, for example:

  • Better integration with other things, like the build system's testing facilities or continuous integration, by virtue of having standardized error codes and status output.
  • More specific assertion macros that help log better error messages.
  • Sometimes easy selection of a single test case via the command line to speed up debugging.

They might also help you test negatives, because currently if you add code that triggers an assert(), your test binary will of course stop working. A lot of test frameworks have a way to deal with this as well, and allow you to specify that you expect certain calls to cause the program to abort. They can then fork and run this call in isolation, and then return a positive result if the forked process aborted, and return a negative result if it didn't.

Test corner cases

You are testing small, regular cases. However, those are the most likely to work correctly. While you should definitely keep these tests, what's maybe even more important is to test the corner cases. For example:

  • Test images with one or both of the dimensions having size zero.
  • Test images with very large dimensions.
  • Test images containing zeroes in some or all of the pixels (hint: integer division by zero might fail)

Test a lot of possible element types

You are only testing images of ints. You probably also want to test unsigned int, float, double, std::complex<>, std::valarray<>, std::boost::rational<> and maybe more.

Of course, you don't want to write duplicate code for all these types, but you can simply make templates out of your test cases, and then just write something to loop over a list of types and call the test case with each type. You might even want to pass unity values to the test cases, so something like this could be done:

void imageElementwiseAddTest(auto unity)
{
    auto test = TinyDIP::Image(10, 10, unity); // Thanks, CTAD!
    test += test;
    ...
}

void runTests() {
    [](const auto&... args) {
        (imageElementwiseAddTest(args), ...);
        ...
    } (
        int(1),
        float(1),
        double(1),
        std::complex<double>(1, 0);
        ...
    );
}

Check that the result is exactly as expected

Currently you only check whether the operations work, but you are not checking the results. If you subtract an image from itself, you would expect all elements to be zero. You currently print the results, and perhaps you are looking at the output and checking it manually? A good test suite should of course automate this. So don't print the output, but check that it is all zeroes, and report an error if not.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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