Workaround for data member initialization happening after base class construction

Question

I would have liked to do something like this:

template<typename T>
class RasterView {
    T* data;
    unsigned stride;
    RasterView(T* data, unsigned stride)
        : data(data)
        , stride(stride)
    {}
public:
    T& operator()(int j, int i) { // hot path
        return data[static_cast<unsigned long>(stride) * i + j];
    }
    RasterView<T> subView(int j, int i) {
        return RasterView<T>(data + static_cast<unsigned long>(stride) * i + j, stride);
    }
    // ...
}

template<typename T>
class Raster : public RasterView<T> {
    std::vector<T> buffer;
public:
    Raster(unsigned w, unsigned h, T defaultPixel)
        : buffer(w*h, defaultPixel)     // construct buffer first because we need...
        , RasterView<T>(buffer.data(), w)  // ... to pass this ptr to the base class constr
    {}
    // ...
}

but of course base class construction happens before member variable initialization, so the pointer buffer.data() isn't available at the time the base class is being constructed. Does anyone have any advice or know of a trick I could use to work around this without adding too much complexity to the implementation?

base class could have a `getData` virtual method that the derived class must implement — 463035818_is_not_an_ai, Dec 30 '22 at 13:24
@463035818_is_not_a_number `RasterView::operator()` is a hot path. Would this come with significant performance overhead? — Museful, Dec 30 '22 at 13:34
yes virtual dispatch has a cost. It is very low, but as `operator()` isnt doing much else, it might be significant. I was a little confused by usage of inheritance in your code, but as you dont want virtual methods, it seems like "normal" inheritance wasnt the right way to begin with. Looks like good candidate for CRTP — 463035818_is_not_an_ai, Dec 30 '22 at 13:40
One option is to assign to the base class members in the body of the constructor of the derived class. This relies on the base class member being accessible to the derived class (e.g. `protected` instead of `private`) or the base class providing a suitable setter. It would work since the body of the constructor is executed after ALL bases and members are initialised. The down-side (apart from the fact that what you're trying to do is not a good idea) is that the base class members will be initialised/constructed first, then reassigned. — Peter, Dec 31 '22 at 10:54
@Peter That's what I ended up resorting to, and I feel quite happy with it now. But I'm really interested to know more about why you say what I'm doing is not a good idea. Would you be willing to elaborate a little for me? — Museful, Dec 31 '22 at 15:30
The reason I consider it's not a good idea is that you're setting things up so a pointer in a base class is pointing at data that is managed by (a member of) a derived class. That sort of dependency tends to make things difficult to manage. Consider what happens, for example, if the destructor of `RasterView` does `delete data` - given that `data` is set to be equal to the `buffer.data()`, where `buffer` is a `std::vector` that ALSO manages its data. Why not have a `std::vector` as a member of `RasterView` and avoid need for `buffer` in `Raster`? — Peter, Dec 31 '22 at 18:06
@Peter `RasterView` was created as a separate type from `Raster` for two reasons (1) it allows for an efficient `RasterView::subView()` and (2) it allows me to create alternate implementations backing the `RasterView` interface with other kinds of memory buffers, (such as OpenCV's `cv::Mat::data` for example) to avoid copying the pixel data. `buffer` is not in `RasterView` for the same reasons. I basically never use `delete` unless I'm implementing some sort of smart pointer. I would be interested to know what you think of the design choice in light of this rationale. — Museful, Jan 01 '23 at 16:19
@Museful No happier. You're implementing a design in which your base class is tightly dependent on how derived classes sets things up. Those things tend to avidly defended by developers - like you now - when doing initial development, because it seems easier when all details are in mind. It is cursed when someone in future - who no longer has all details in mind that you now have - has to maintain the program. There are few experiences more humbling than spending hours or days tracking down a bug, wondering "what idiot wrote this?" and realising "Oh, it was me ....". — Peter, Jan 02 '23 at 02:07
@Peter Can you suggest a different design that would achieve these objectives in any simpler/better way? — Museful, Jan 02 '23 at 11:08
@Museful - There are several options. One would be to make `subView()` a (possibly protected) virtual member function of the base, with arguments representing the `data` and `stride` members. Provide an definition of `RasterView::subView()` that does your preferred implementation. Then have derived classes manage calling that function (e.g. if they manage data in a `vector`, pass the `.data()` member). Then the derived class design can determine if (and if so, how) it overrides that default implementation. — Peter, Jan 02 '23 at 13:37
@Peter `RasterView::subView(j,i)` is an important part of the public interface. It's the reason for my design. Is it possible to do what you suggest without breaking the interface? — Museful, Jan 02 '23 at 21:03
@Museful Sure. Either make the `RasterView::subView(i,j)` a pure virtual (which forces the derived classes to override it, and allows them to do what they like to comply with the interface) or change the implementation of `subView()` so it calls a protected/virtual helper function. Seems however, that you've gone from a statement in the question "I would have liked to do ..." to "... is an important part of the public interface". Which smells like constructing requirements to paper over an approach you want, rather than one to suit the task. — Peter, Jan 03 '23 at 02:04
@Peter Oh I would have done that from the start if it were possible without also introducing virtual dispatch within/for `RasterView::operator()` (a hot path). Do you know of a way? — Museful, Jan 03 '23 at 09:34

HolyBlackCat · Accepted Answer · 2022-12-30T15:01:15.550

The straighforward solution is to move buffer to a new base class, and inherit from it before RasterView.

But I would rather turn RasterView into a CRTP base class:

#include <cstddef>
#include <vector>

template <typename Derived, typename T>
class RasterView
{
public:
    T &operator()(int j, int i)
    {
        auto buffer = static_cast<Derived *>(this)->data();
        auto stride = static_cast<Derived *>(this)->stride();
        return buffer[stride * i + j];
    }
};

template <typename T>
class Raster : public RasterView<Raster<T>, T>
{
    std::vector<T> buffer;
    std::size_t width;

public:
    Raster(std::size_t w, std::size_t h, T value = {})
        : buffer(w*h, value), width(w)
    {}
    
    T *data()
    {
        return buffer.data();
    }
    std::size_t stride() const
    {
        return width;
    }
};

int main()
{
    Raster<int> r(3,4,42);
    r(1,2) = 3;
}

any advice for how to add back RasterView::subView()?

Like this:

#include <cstddef>
#include <vector>

template <typename T>
class RasterView;

template <typename Derived, typename T>
class RasterBase
{
  public:
    T &operator()(int j, int i)
    {
        auto buffer = static_cast<Derived *>(this)->data();
        auto stride = static_cast<Derived *>(this)->stride();
        return buffer[stride * i + j];
    }

    RasterView<T> sub_view(int j, int i);
};

template <typename T>
class RasterView : public RasterBase<RasterView<T>, T>
{
    T *ptr = nullptr;
    std::size_t pixel_stride;

  public:
    RasterView(T *ptr, std::size_t pixel_stride) : ptr(ptr), pixel_stride(pixel_stride) {}

    T *data()
    {
        return ptr;
    }
    std::size_t stride() const
    {
        return pixel_stride;
    }
};

template <typename Derived, typename T>
RasterView<T> RasterBase<Derived, T>::sub_view(int j, int i)
{
    auto buffer = static_cast<Derived *>(this)->data();
    auto stride = static_cast<Derived *>(this)->stride();
    return RasterView<T>(buffer + stride * i + j, stride);
}

template <typename T>
class Raster : public RasterBase<Raster<T>, T>
{
    std::vector<T> buffer;
    std::size_t width;

  public:
    Raster(std::size_t w, std::size_t h, T value = {})
        : buffer(w*h, value), width(w)
    {}
    
    T *data()
    {
        return buffer.data();
    }
    std::size_t stride() const
    {
        return width;
    }
};

int main()
{
    Raster<int> r(3,4,42);
    r(1,2) = 3;
    RasterView<int> view = r.sub_view(1,1);
    view(1,2) = 3;
}

Do you have any advice for how to add back `RasterView::subView()`? The intent is to be able to pass around `raster` and `raster.subView(j0, i0)` (representing a window into the raster) interchangeably. (For simplicity I had excluded window dimensions because I don't think they are essential.) — Museful, Dec 30 '22 at 14:52
@Museful Added a solution. I'd also add a conversion operator from `RasterBase` to `RasterView`. — HolyBlackCat, Dec 30 '22 at 15:03
I see that makes it more complicated. Would the straightforward solution you mentioned first (i.e. moving `buffer` to a new base class and inheriting from it first) work for this (and without adding overhead on calls to the second base class's members)? — Museful, Dec 30 '22 at 15:13
@Museful Referring to a base class has no overhead (if it's not `virtual`, at least). It's exactly the same as what you have in the question, but with the right initialization order. — HolyBlackCat, Dec 30 '22 at 15:16

Stack Danny · Answer 2 · 2022-12-30T14:37:16.213

0

If you were to provide a default constructor to RasterView (or simply adding default values here), than you could call call the constructor inside the Raster constructor, effectively delaying the call:

template<typename T>
class RasterView {
public:
    T* data;
    unsigned stride;
    RasterView(T* data = nullptr, unsigned stride = 0u) : data(data), stride(stride) 
    {
        
    }
};

template<typename T>
class Raster : public RasterView<T> {
    std::vector<T> buffer;
public:
    Raster(unsigned w, unsigned h, T defaultPixel) : buffer(w* h, defaultPixel)
    {
        RasterView<T>::operator=(RasterView(buffer.data(), w));
    }
};

edited Dec 30 '22 at 14:37

answered Dec 30 '22 at 13:48

Stack Danny

7,754
2
26
55

This doesn't work. You don't call the constructor on your object, you construct a temporary. – HolyBlackCat Dec 30 '22 at 14:06
@HolyBlackCat check my edit, adding `this->` will work here. – Stack Danny Dec 30 '22 at 14:31
[Only MSVC accepts this](https://gcc.godbolt.org/), this is some non-standard stuff. The solution is `RasterView::operator=(RasterView(buffer.data(), w));`. Or `static_cast &>(*this) = RasterView(buffer.data(), w);`. – HolyBlackCat Dec 30 '22 at 14:35
peculiar syntax weirdness. – Stack Danny Dec 30 '22 at 14:48
It makes sense under the hood. A constructor creates a new object, so you can't call it on an existing object. Instead you can assign to the object. Normally you would do `*this = ...`, but since the derived class has its own implicitly generated `operator=`, it shadows the base one, so you have to jump through hoops. – HolyBlackCat Dec 30 '22 at 14:51
I see, thanks for the insight and important attention to detail. – Stack Danny Dec 30 '22 at 14:55

Workaround for data member initialization happening after base class construction

2 Answers2