6

I am currently designing an API and I am not sure whether my functions should take shared_ptr or weak_ptr. There are widgets that contain viewers. The viewers have a function add_painter which adds a painter to the viewer. When a viewer needs to redraw, it uses its painters to draw into a buffer and displays the result. I came to the conclusion that the viewers should hold the painters using weak_ptr:

  • A painter may be used in multiple viewers, so the viewer cannot own the painter.
  • Deleting the painter should remove it from the viewer. This way, users do not need to remember that they have to call a remove_painter function.

There may be different kind of viewers, so they are hidden behind an interface. What signature is best for the add_painter function in the interface?

Should I directly use void add_painter(weak_ptr<Painter> const& p)? This implies that the concrete implentations store the painters using weak_ptr, but I cannot enforce this: An implementation could just do painters.push_back(weak_ptr.lock()) and store a shared_ptr.

Should I use void add_painter(shared_ptr<Painter> const& p) instead? This implies that the viewers hold strong references, so that deleting a painter does not necessarily remove it from the viewer.

I also considered storing the painters directly in the interface class, but then it is no real interface anymore, is it?

pschill
  • 5,055
  • 1
  • 21
  • 42
  • 1
    I'd say https://softwareengineering.stackexchange.com would be a better place for this question. – Kane Jan 31 '18 at 08:09
  • Okay, should I reask it there or can you move it somehow? – pschill Jan 31 '18 at 08:12
  • Easier if you move it. Software engineering isn't on the vote to move list, so you'd have to wait for someone with vast godly might to migrate it. That said, I think it's a reasonable fit for this site. Far as I'm concerned, this is a good question even if it might be crossing into opinion based. I want to see what some of the gurus bring to the table. I don't have anything concrete on this one, so I'd just be opinion and I'm going to shut up now. What the heck am I doing up anyway? At this hour any advice I gave would be questionable at best. – user4581301 Jan 31 '18 at 08:31
  • No, this was just my personal suggestion. Maybe you consider the SE section next time. Anyway, +1 for a good question. – Kane Jan 31 '18 at 08:36
  • Why does your API use `shared_ptr` on the first place? `shared_ptr` implies worst possible ownership pattern, which typically can be avoided. – user7860670 Jan 31 '18 at 09:00
  • @VTT In the question I give reasons for `weak_ptr`. And using `weak_ptr` means using `shared_ptr` somewhere else. – pschill Jan 31 '18 at 09:07
  • I mean avoiding both `shared_ptr` and `weak_ptr`. It seems that people often use `shared_ptr` to "be on the safe side". – user7860670 Jan 31 '18 at 09:10
  • @Kane when referring other sites, it is often helpful to point that [cross-posting is frowned upon](https://meta.stackexchange.com/tags/cross-posting/info) – gnat Jan 31 '18 at 09:46

1 Answers1

1

You should not try to mitigate the Observer pattern with smart pointers and definitely you should avoid a situation when a client (View) can harass the server by converting the weak pointer to a shared pointer and storing it indefinitely barring it from being released by the server.

You should really consider the classic Observer pattern here requesting View to provide a painter_destroyed callback function. It may be an annoyance but also gives the client an opportunity to implement some additional actions once the painter is destroyed. Otherwise finding that the painter exists no more just when one wants to use it may be quite irritating and affect overall program performance.

jszpilewski
  • 1,632
  • 1
  • 21
  • 19
  • This means that I have to invert the relationship between viewer and painter, right? Instead of calling `add_painter` on the viewer I do something like `add_viewer` / `add_observer` on the painter. – pschill Jan 31 '18 at 09:38
  • 1
    I think it remains up to your taste. add_painter may return a pointer to callback function – jszpilewski Jan 31 '18 at 09:48
  • This function should be called before the painter is destroyed, giving the observer a chance to gracefully cease to access it. Naming it e.g. `on_destroying_painter` would be more logical then. (Sorry for stating the obvious.) – Arne Vogel Jan 31 '18 at 16:58