Skip to main content
a few run-on sentences and a missing word
Source Link
JDługosz
  • 11.7k
  • 19
  • 40

This constructor becan be easily implemented in terms of std::copy. You do need to add a test here to verify that the input array has the right number of elements considering newWidth and newHeight.

It is not necessary to use this-> to access members. Some people like it, but I think it is superfluous.

You don't need to forward declare class Image. You include its header file, so the class is already declared.

Functions here are declared out of order: functions use other functions that are declared later in the file. Somehow this works for you, I think, because templates are instantiated when used. It would be better practice to declare normalDistribution1D before gaussianFigure2D, and gaussianFigure2D2 before gaussianFigure2D. Note that class methods can be declared in any order -- the compiler will first collect all member names (collect the full definition of the class) before attempting to compile each of the function bodies. The same is not true for free functions such as the ones we're talking about here.

This constructor be easily implemented in terms of std::copy. You do need to add a test here to verify that the input array has the right number of elements considering newWidth and newHeight.

It is not necessary to use this-> to access members. Some people like it, I think it is superfluous.

You don't need to forward declare class Image. You include its header file, the class is already declared.

Functions here are declared out of order: functions use other functions that are declared later in the file. Somehow this works for you, I think because templates are instantiated when used. It would be better practice to declare normalDistribution1D before gaussianFigure2D, and gaussianFigure2D2 before gaussianFigure2D. Note that class methods can be declared in any order -- the compiler will first collect all member names (collect the full definition of the class) before attempting to compile each of the function bodies. The same is not true for free functions such as the ones we're talking about here.

This constructor can be easily implemented in terms of std::copy. You do need to add a test here to verify that the input array has the right number of elements considering newWidth and newHeight.

It is not necessary to use this-> to access members. Some people like it, but I think it is superfluous.

You don't need to forward declare class Image. You include its header file, so the class is already declared.

Functions here are declared out of order: functions use other functions that are declared later in the file. Somehow this works for you, I think, because templates are instantiated when used. It would be better practice to declare normalDistribution1D before gaussianFigure2D, and gaussianFigure2D2 before gaussianFigure2D. Note that class methods can be declared in any order -- the compiler will first collect all member names (collect the full definition of the class) before attempting to compile each of the function bodies. The same is not true for free functions such as the ones we're talking about here.

added 1470 characters in body
Source Link
Cris Luengo
  • 7k
  • 1
  • 15
  • 37

This constructor initializes the array with a constant value:

        Image(const int width, const int height, const ElementT initVal):
            width(width),
            height(height),
            image_data(width * height)
        {
            this->image_data = recursive_transform<1>(this->image_data, [initVal](ElementT element) { return initVal; });
            return;
        }

Use the std::vector constructor for this:

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

If you further add a default value to the last parameter (const ElementT initVal = {}) then you don't need the previous constructor (Image(const size_t width, const size_t height)).

        Image(const std::vector<ElementT>& input, size_t newWidth, size_t newHeight)
        {
            this->width = newWidth;
            this->height = newHeight;
            this->image_data = recursive_transform<1>(input, [](ElementT element) { return element; });   //  Deep copy
        }

This constructor be easily implemented in terms of std::copy. You do need to add a test here to verify that the input array has the right number of elements considering newWidth and newHeight.

The implementation of at doesn't check bounds. I would suggest adding assert statements for bounds, to catch bugs in debug mode. A release build would then not test bounds.

Functions here are declared out of order: functions use other functions that are declared later in the file. Somehow this works for you, I think because templates are instantiated when used. It would be better practice to declare normalDistribution1D before gaussianFigure2D, and gaussianFigure2D2 before gaussianFigure2D. Note that class methods can be declared in any order -- the compiler will first collect all member names (collect the full definition of the class) before attempting to compile each of the function bodies. The same is not true for free functions such as the ones we're talking about here.

Again you have way too many headers included here. The

This is very complex code in, which seems out of place with the relatively straight-forward C++ code elsewhere. With the changes suggested above, you don't need this file is not used in your program, as far as I can tellat all.

The implementation of at doesn't check bounds. I would suggest adding assert statements for bounds, to catch bugs in debug mode. A release build would then not test bounds.

Again you have way too many headers included here. The code in this file is not used in your program, as far as I can tell.

This constructor initializes the array with a constant value:

        Image(const int width, const int height, const ElementT initVal):
            width(width),
            height(height),
            image_data(width * height)
        {
            this->image_data = recursive_transform<1>(this->image_data, [initVal](ElementT element) { return initVal; });
            return;
        }

Use the std::vector constructor for this:

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

If you further add a default value to the last parameter (const ElementT initVal = {}) then you don't need the previous constructor (Image(const size_t width, const size_t height)).

        Image(const std::vector<ElementT>& input, size_t newWidth, size_t newHeight)
        {
            this->width = newWidth;
            this->height = newHeight;
            this->image_data = recursive_transform<1>(input, [](ElementT element) { return element; });   //  Deep copy
        }

This constructor be easily implemented in terms of std::copy. You do need to add a test here to verify that the input array has the right number of elements considering newWidth and newHeight.

The implementation of at doesn't check bounds. I would suggest adding assert statements for bounds, to catch bugs in debug mode. A release build would then not test bounds.

Functions here are declared out of order: functions use other functions that are declared later in the file. Somehow this works for you, I think because templates are instantiated when used. It would be better practice to declare normalDistribution1D before gaussianFigure2D, and gaussianFigure2D2 before gaussianFigure2D. Note that class methods can be declared in any order -- the compiler will first collect all member names (collect the full definition of the class) before attempting to compile each of the function bodies. The same is not true for free functions such as the ones we're talking about here.

Again you have way too many headers included here.

This is very complex code, which seems out of place with the relatively straight-forward C++ code elsewhere. With the changes suggested above, you don't need this file at all.

deleted 1 character in body
Source Link
Cris Luengo
  • 7k
  • 1
  • 15
  • 37

Proper English spelling would be "Developpeded by Jimmy Hu".

Proper English spelling would be "Developped by Jimmy Hu".

Proper English spelling would be "Developed by Jimmy Hu".

added 1 character in body
Source Link
Cris Luengo
  • 7k
  • 1
  • 15
  • 37
Loading
Source Link
Cris Luengo
  • 7k
  • 1
  • 15
  • 37
Loading