2

What I'm asking is what is better practice with regards to the member "mylist":

declaring mylist like this:

class MyClaz {
  std::list<string> mylist;
  ...
}

or declaring it like this:

class MyClaz {
  std::list<string> *mylst;
  MyClaz();
  ...
}

MyClaz::MyClaz() {
  myList = new std::list<string>();
}
// plus destructor to delete myList

In the first case, will mylist get automatically allocated when an instance of MyClaz is created? Will it get cleaned up properly when MyClaz is destroyed?

Is there any reason to use the second case ever? It seems more dangerous.

marathon
  • 7,881
  • 17
  • 74
  • 137
  • One of many related posts about the topic: http://stackoverflow.com/questions/712639/understanding-the-meaning-of-the-term-and-the-concept-raii-resource-acquisiti/713773#713773 – πάντα ῥεῖ Jun 30 '14 at 20:35

6 Answers6

10

Use pointers sparingly, only when needed. There are multiple advantages of not having pointers, including not having to manually manage the lifetime of the objects and data locality. The question should be whether there is any reason to use the second case...

David Rodríguez - dribeas
  • 204,818
  • 23
  • 294
  • 489
2

To answer your first question, yes, mylist will be automatically allocated and automatically destroyed that is why the second method is not good in general.

Note the automatically created mylist will be an empty string though.

CS Pei
  • 10,869
  • 1
  • 27
  • 46
2

is it good practice to make those member objects pointers?

Mostly no1. With C++ standard library containers, especially no.

sizeof(std::list<string>) is very small, and the class manages it's own lifetime. Every list element will be destroyed by the list destructor, which will be called automatically by your automatically created destructor.

Creating container with new is completely missing the point.

In the first case, will mylist get automatically allocated when an instance of MyClaz is created? Will it get cleaned up properly when MyClaz is destroyed?

Yes, and yes.

Default constructor of mylist gets called by the constructor of MyClaz, and destructor of mylist gets called by the destructor of MyClaz (if you don't write one - the compiler creates one for you)

1 - there are some reasons you should use pointers (e.g. PIMPL, polymorphism), but in these cases you use smart pointers (std::shared_ptr, std::unique_ptr), not raw pointers (T*).

milleniumbug
  • 15,379
  • 3
  • 47
  • 71
  • when you say "every list element will be destroyed by the destructor", do you mean some wrapper managed internally by the list that wraps each item inserted, or do the actual items you put in the list get destroyed automatically, too? – marathon Jun 30 '14 at 20:41
  • 1
    @marathon You're creating an object in the constructor, destroying it in the destructor (to avoid leaking resources, you should also handle copying and moving - and you probably forgot these). There is no reason `std::list` cannot do the same thing. (and it does - no worries) – milleniumbug Jun 30 '14 at 20:48
1

There is no point in in "new"ing your container, because then you have to worry about deleting it in the destructor. Best way is to use method1. Maybe you might want to have an std::list of references instead of actual objects.

Also, if you are using STL it is better to resort to smart pointer (e.g. std::shared_ptr), that way you do not have to worry about calling delete on it either.

std::shared_ptr<std::list<string>> mylist;
//in constructor
mylist = std::make_shared<std::list<string>>();
//in destructor it will be deleted automatically

//if you want to have a list of references rather than objects themselves
std::list<std::shared_ptr<string>> mylist;

hope it was helpful.

santahopar
  • 2,933
  • 2
  • 29
  • 50
1

In most cases, it makes sense to use an object instead of pointers. However, there are exceptions where it makes sense to use pointers. It's good to know the exceptions.

The most compelling reason I have found is that it is expensive to have a list as a member variable when the list is expected to be empty most of the time. There are memory and time costs of constructing a list that can be avoided for most objects.

Here's a simplified example of something that is often found in CAD/CAE applications.

class Attribute;

class Entity
{
   std::list<Attribute*>* attributes;
};

class Attribute: public Entity
{
};

Most Entitys will not have Attributes but many will. These are use specified data as far as Entity is concerned. A Part, which is likely to be derived from Entity, may have visual properties such as Color, material properties such as Density. However, a Face of the Part may have its own Color. In the absence of its own Color, a Face inherits its containing Part's Color.

The key reason for using a std::list<Attribute*>* instead of std::list<Attribute*> is that it can be expensive to have a list for each object when most objects don't need it. The pointer will be assigned to NULL in the constructor. When an object needs to have an item in the list, a list will be allocated from the heap and used henceforth.

The additional logic comes with the additional maintenance burden but the cost can be justified for the gains.

Update

For various reasons, we are still required to work with Microsoft Visual Studio 2008 on Windows 7 (64 bit). On that platform,

sizeof(std::list<Attribute*>) is 48

sizeof(std::vector<Attribute*>) is 40

sizeof(std::vector<Attribute*>*) is 8.

With g++ 4.7.3 (32-bit), the sizes are:

sizeof(std::list<Attribute*>) is 8

sizeof(std::vector<Attribute*>) is 12

sizeof(std::vector<Attribute*>*) is 4.

Depending on your platform, the additional cost of objects vs pointers can be prohibitive. A judgement call has to be made based on the numbers on a platform.

R Sahu
  • 204,454
  • 14
  • 159
  • 270
  • "it is expensive to have a list as a member variable when the list is expected to be empty most of the time." This is not the case. The default constructor of `std::list` does no dynamic allocations in gcc and Microsoft C++ standard library. – milleniumbug Jun 30 '14 at 21:20
  • @milleniumbug, you are right about `gcc`. The picture is quite different for VS. `sizeof(std::list)` is 48, `sizeof(std::vector)` is 40, while `sizeof(std::vector*)` is only 8 (for 64 bit). The corresponding numbers for gcc are 8, 12, and 4 (for 32 bit). – R Sahu Jun 30 '14 at 21:23
  • (1) My point that "no dynamic allocations take place" still stands. (2) Writing several members is very cheap compared to dynamic allocation. If you have 100 Entities, and only one has created a list, your program is still slower. (3) `class Attribute; std::cout << sizeof(std::list) << " " << sizeof(std::vector) << " " << sizeof(std::list*);` on Visual Studio 2013 32-bit Debug configuration says: `12 16 4`, and Release version says `8 12 4`. This is because debug version checks more, and this checking requires storing some info in the list. – milleniumbug Jun 30 '14 at 21:36
  • @milleniumbug Please see my update. The key point is that a team has to make a judgement call based on the relative costs. We had to make the switch from objects to pointers at my work due to the additional cost. – R Sahu Jun 30 '14 at 21:51
  • Just curious - what are the sizes on Release? – milleniumbug Jun 30 '14 at 22:05
  • Worse than Debug. 56, 48, 8. – R Sahu Jun 30 '14 at 22:06
1

When you think pointers, think lifetime and cost. When you have a pointer, you're really saying that whatever the pointer points to may or may not exist when you want to use it and that your code will handle both cases. While this enables a lot of flexibility and can improve performance, it comes at a cost.

  • You can no longer assume what is pointed to exists and need code to handle both cases - when it exists and when it does not.
  • You now have to be very careful about how you use the STL - for example: comparisons will be done on the pointers instead of what they point to.
  • Your code is now vulnerable to a whole class of pointer related bugs.

C++ is a language that lets you do almost anything. It assumes you know what you're doing.

So the answer to your question is, use pointers only if you know why you need them.

If, in your head you're thinking, "Maybe pointers might be better", then the answer is no, they're not. Why? Because you're not clear on the cost of using pointers and in the long run this will do more harm than good to your code. Remember, again, C++ assumes you know what you're doing.

On the other hand, if you decide not to use pointers, then what should you use? Use it exactly the way you would:

class MyClaz {
  std::list<std::string> mylist; //I assume you meant std::string here
  ...
}

For now, trust the compiler. Don't worry about copying by value. Worry about your code and how well and how clearly it does what you want it to do. That's good practice.

So in general, good practice is to only make a change when you have a sound reason for doing so.

Carl
  • 43,122
  • 10
  • 80
  • 104