Add missing defaulted ctor, dtor and operator= for Histogram

[milianw]

The histogram class has many constructors, but was missing all of the default constructors. This made it much harder to use, one would not be able to cmopile std::vector<dip::Histogram>(5) e.g.

Personally I don't see a reason why it should be forbidden to construct an empty histogram.

@crisluengo I also see that the other ctors are explicitly checking if the input image is forged, and throw an exception otherwise. Why is that? I also find that quite unexpected. To me, it sounds like a valid operation to do, and the result should be an empty histogram...

[crisluengo]

A default-constructed histogram would have a raw image inside, but all the histogram code assumes that the image is forged, it doesn't check for a valid data pointer before dereferencing it.

So a default-constructed histogram must allocate image data. Perhaps a 1D histogram using a default-constructed Configuration, initialized to zeros.

If we were to allow creating a raw histogram, we'd need to add a test to all histogram functions, and we'd need to add functions such as IsForged(), SetConfiguration(), and Forge(). This would be a lot of work for no benefit, a raw histogram is quite useless.

Alternatively, do std::vector<std::unique_ptr<dip::Histogram>>

---

A histogram computed from a raw image could be a histogram with all data set to zero as well. There's no reason not to do it this way. The current "throw on a raw image as input" is the normal behavior for all functionality in the library, and is behavior inherited from the original DIPlib. I'm not sure it's useful to allow a raw image as input, but I don't see a problem with it.

[milianw]

Please reconsider. To me, a dip::Histogram (and dip::Image for that matter) is just a container. Containers have a defined state when they are empty. As a user of the API, I find it very inconvenient that I have to push a dip::Histogram into a pointer (and pay the cost for the additional allocation). Imagine you'd have to do the same when you want to support empty std::string or std::vector or any other kind of container structure.

Furthermore, I consider this to be very inconsistent within DIPlib: Why can I have raw images, but not histograms?

Put differently: Would you be OK if I scan through the API to add exceptions to all functions that operate on a histogram, to bail out when they are empty, i.e. not-forged? I don't think it's too much work.

And again: an empty histogram is just as "useless" as an empty image. There are simply situations where an operation to read an image fails. Right now, it is simply impossible to write concise code like this:

struct ImageAndHistogram
{
    dip::Image image;
    dip::Histogram histogram;
    explicit operator bool() const { return image.IsForged(); }
};
ImageAndHistogram loadImageAndHistogram(const std::string &file)
{
    if (!open(file)) return {};
    ...
}

[crisluengo]

OK. So we'd have a raw dip::Histogram that you can't use for anything, it's just meant as a placeholder, you can only copy or move another histogram into it.

[crisluengo]

By the way, the copy/move constructor/assignment and the destructor are not needed, they should be automatically generated by the compiler.

[milianw]

Yes, exactly - pretty much the same as an empty string. The difference is that the string also offers API to resize it, which the histogram does not have. But again, I don't really think that's needed - at least not for my purposes.

Regarding the auto-generation: dip::Histogram has lots of constructors, so the default ones are not generated. See e.g.: https://godbolt.org/z/zYn3v9EsY

[milianw]

Regarding the auto-generation: dip::Histogram has lots of constructors, so the default ones are not generated. See e.g.: https://godbolt.org/z/zYn3v9EsY

Ah I see - I only need the defaulted default constructor :) will update

[marc-kdab]

From a higher-level POV, the default ctor of a class need not establish a valid value [EoP, p.8]. It can just establish the partially-formed state, where the only valid operations are destruction and the assignment of a new value. Cf.

 int i = 0; // well-defined
 int j;      // partially-formed

yet there's no is_valid(int &) in the language. Likewise, a default-constructed Histogram can be partially-formed, with all member functions being UB, except assignment and destruction, and no checking in the members for the partial state (except maybe assert()ing).

See https://duckduckgo.com/?q=marc+mutz+partially-formed&ia=web for more info.

[EoP] http://elementsofprogramming.com/eop_bluelinks.pdf

[crisluengo]

@marc-kdab You are right, of course. But one design decision for DIPlib was that we’d be able to map all functions to an interactive language. Doing the wrong thing in an interactive language must lead to an error message, not a crash. So if an object can be partially-formed, we need all functions to check for that state and throw a (meaningful) error, rather than possibly dereferencing an invalid pointer.

[crisluengo]

I've pushed this commit, with a bunch of tests added for partially-formed objects, in 125880ad.

[milianw]

Thanks Cris, much appreciated!