A better way to store a structure

Question

Here is a problem:

class DrawingsContainer {
 public:
  std::vector<std::pair<std::string, std::shared_ptr<Drawing>>> getDrawings() { return drawings; }
 private:
  std::vector<std::pair<std::string, std::shared_ptr<Drawing>>> drawings;
};

I have a variable

std::vector<std::pair<std::string, std::shared_ptr<Drawing>>> drawings

I'm using std::pair because I need the items I put into the vector to be in the same order I've placed them so std::map is a no no. (I use this container to draw everything on the screen). std::string is there for search purposes.

class Drawing {
 public:
  // only one can be active, thus both are initialized as nullptr at the start and one type is overwritten on creation
  explicit Drawing(
      std::string act,
      std::unique_ptr<sf::Text> txt = nullptr,
      std::unique_ptr<sf::RectangleShape> rectShape = nullptr
  ) {
    active = std::move(act);
    text = std::move(txt);
    rect = std::move(rectShape);
  }
  std::string getActive() { return active; }
  std::shared_ptr<sf::Text> getText() { return text; }
  std::shared_ptr<sf::RectangleShape> getRect() { return rect; }
 private:
  std::string active;
  std::shared_ptr<sf::Text> text;
  std::shared_ptr<sf::RectangleShape> rect;
};

I don't feel like this is a good solution, because what if I use more than sf::Text and sf::RectangleShape classes in the structure ? feels dirty.

Ideally I would like to have this kind of structure:
[["arena", sf::RectangleShape], ["optionsButton", sf::Text]].
where sf::RectangleShape and sf::Text are derivatives of base class sf::Drawable, of course they have different methods which I need to call. here is a nice explanation demonstrating the hierarchy (first picture) - https://www.sfml-dev.org/documentation/2.4.2/classsf_1_1Drawable.php

So I'm wondering if there is a better way to do this.

Seems like a [CodeReview](https://codereview.stackexchange.com/) question — Neijwiert, Feb 21 '18 at 07:31
It's kind of unclear what would you like to enhance in your code... Could you be more specific on that? — W.F., Feb 21 '18 at 07:31
you could use `sf::Drawable` in combination with `dynamic_cast` or you add a label to the `sf::Drawable` and use `static_cast` to get the real class of the pointer — gotocoffee, Feb 21 '18 at 07:34
Note: `getDrawings()` should be const and return be reference to avoid copy. — Jarod42, Feb 21 '18 at 12:57
I tried posting on CodeReview and got downvoted and flagged for a "off-topic" :) @gotocoffee thanks, i will try that. @Jarod42 you`re right, thanks ! — Povilas Gintutis, Feb 23 '18 at 06:42

score 2 · Answer 1 · answered Feb 21 '18 at 23:09

2

It seems that you can use regular polymorphism:

struct Drawing
{
    std::string name;
    std::shared_ptr<sf::Drawable> drawable;
};

So

class DrawingsContainer
{
public:
    const std::vector<Drawing>& getDrawings() const { return drawings; }
private:
    std::vector<Drawing> drawings;
};

answered Feb 21 '18 at 23:09

Jarod42

203,559
14
181
302

And how would i then access the methods of derived classes ? i mean, i need ```sf::Text```'s functionality, but how can the base class know about it's derivatives` methods ? – Povilas Gintutis Feb 23 '18 at 06:40
You need `dynamic_cast` (or add extra layer to allow visitor). – Jarod42 Feb 23 '18 at 08:30

score 1 · Answer 2 · edited Feb 24 '18 at 10:45

1

Use typedef to reduce the long ugly typenames. Since there may be only one instance of the drawing container, singleton pattern may be used like so.

struct node_t
{
  sf::Text text;
  sf::RectangleShape rect;    
};

class Drawing {
 private:
    std::vector<std::shared_ptr<node_t>> tools_;
};

typedef std::vector<std::pair<std::string, std::shared_ptr<Drawing>>> container_t;

class DrawingsContainer {
 public:
  static const std::unique_ptr<DrawingsContainer>& GetInstance() 
  { 
      static std::unique_ptr<DrawingsContainer> ptr(new DrawingsContainer());      
      return ptr; 
  }
 private:
   DrawingsContainer() {}
  container_t drawings_;
};

edited Feb 24 '18 at 10:45

Povilas Gintutis

170
1
9

answered Feb 21 '18 at 07:39

seccpur

4,996
2
13
21

yeap, the singleton idea is correct, i will try to implement this when i have a bit more time. Either way, i didn't know one could ```typename``` , thanks ! :D – Povilas Gintutis Feb 23 '18 at 06:46
Please don't make something a Singleton just because you *can*. The pattern has advantages and disadvantages. You currently don't need the advantages (no second instance, guaranteed) but instead use the disadvantages (global variable). That is a misuse of the pattern. – nvoigt Feb 23 '18 at 13:28
@nvoigt why should i not use it in this case ? i do need only one instance, because having more than one would be a nuisance (passing the object by reference everywhere, creating additional dependencies or accidentally creating a new instance (which would create bugs)). Would it really be that bad to have a global object, which is used in almost every other class ? – Povilas Gintutis Feb 24 '18 at 06:22
@PovilasGintutis [Yes](https://stackoverflow.com/questions/484635/are-global-variables-bad). [Indeed](https://softwareengineering.stackexchange.com/questions/347199/parametrize-methods-vs-global-variables). – nvoigt Feb 24 '18 at 20:01
ok, i understand why it is bad, and my priority is code readability / maintainability, but in my case it just feels like i don't have the downside of using the container globally. I mean it's just a container of drawings which are added and removed. And almost every other class needs to have access to it. I just don't see how this would impede on debugging process, because, you know, call stack. Also i will not be using any advanced stuff like multithreading, so it just feels like a lot more efficient way. – Povilas Gintutis Feb 25 '18 at 06:59

A better way to store a structure

2 Answers2