Pop() etiquette

Question

Situation: I'm implementing a list-like container that supports a Pop() function that should return the user the const char* stored at the front of the container. However, I'm not sure as an implementer whether or not I should be returning the original const char* (deleting the node pointer from the container but not calling delete on the const char* itself), or whether I should allocate new memory and return a copy of the element.

From class and projects, I've encountered those who espouse always copying, so that no previously returned references (from getters, etc.) and pointers to the const char* can alter the popped version, but since this approach requires an extra allocation and strcpy, I thought I'd ask whether simply returning the original, undeleted pointer to const char without deleting it is really a problem. Here's a code snippet of the approach WITH allocation and copy (and deleting the original reference afterwards):

const char* LinkedList::PopHeadString()
{
    node* deletehead = head_;
    char* output = NULL;

    if (head_ != NULL && GetHeadType() == STRING) {
        output = new char[strlen(head_->en.data.str) + 1];
        strcpy(output, head_->en.data.str);
        head_ = head_->next;
        delete deletehead->en.data.str;
        delete deletehead;
        --nEntries_;
    }
    return output;
}

Since Pop() is a common container operation, I thought I'd ask what the general approach is.

The solution from the STL's solution is that `pop()` doesn't return anything at all. If the user wants the object, they obtain a reference to it with `back()` -- making a copy, of course, if they need to use the object after calling `pop()`. Also, an STL container of pointers only assumes ownership of the pointers, not of the object pointed to. — , May 14 '12 at 06:32
.. so if I follow the STL convention, assuming ownership of stored (inserted) pointers but not ever calling new/delete myself, then does that also mean that STL copy constructors of containers are shallow copies? For instance, if one of my member functions is supposed to return a new list with all elements after the first one (i.e. GetRemainder), then I would be creating a copy list with pointers pointing to the same dynamic memory addresses as the original? — Erika Electra, May 14 '12 at 07:22
Yes. Containers operate on value semantics, so if you create an object of type `std::list`, it treats the pointers as the values. So if you copy the list, it copies the pointers, and doesn't try to guess what you're using the pointers for. Note this is usually The Right Thing. In my observation, C-style strings are really the only common exception. Of course, the general opinion is that that you shouldn't be using C-style strings in C++: you should be using `std::string`. — , May 14 '12 at 08:05
Thanks for that suggestion. I'd love to use strings, but besides the fact that I've been forbidden from using the STL, I also thought about making my container heterogeneous (with each element possibly being a string, another branch list, or a function pointer to execute on following elements), possibly with a union, in which case it seems like containing pointers might be more efficient than containing objects. Would the shallow copy still be best, though? `typedef union { const char* str; LinkedList* list; int (*proclist)(int, LinkedList); // func to execute on list } content;` — Erika Electra, May 14 '12 at 08:14

score 4 · Accepted Answer · answered May 14 '12 at 06:21

4

If you return the pointer without copying you have to decide who owns the pointer. The one who owns it is responsible for deleting it when it is no longer needed. This can be a very hard problem, especially if your code gets more complex. Copy-semantics are much simpler to reason about.

In your specific example, the first thing to change would be to use std::string instead of const char * to represent strings. If you return an std::string by value, it will take care of the copying for you.

If you really want to prevent the copy, but still manage the lifetime elegantly, you should consider using an std::shared_ptr<std::string> (or boost::shared_ptr<std::string> if you don't have a C++11-compiler). A shared_ptr uses reference-counting to determine when the object it points to should be released, thus taking the burden of manual memory management from you, which is a Good Idea.

answered May 14 '12 at 06:21

Björn Pollex

75,346
28
201
283

That is a very good point -- ownership. I don't know who owns the pointer, and I'm not sure how to answer that question. For example, right now, when a user of the class prepends a new const char* element, I have currently programmed the container to not simply contain the pointer, but rather to copy the pointed-to element. I didn't do this for any particular reason, other than some offhand fear that if I didn't create a new copy for the contained pointer, then the inserted element might go out of scope before the container (i.e. container is passed around even after calling function returns) – Erika Electra May 14 '12 at 06:42
If the container creates a copy when an element is added, then you can consider it the owner of the object. – Björn Pollex May 14 '12 at 06:46
...but now that I think about it further, memory declared on the heap shouldn't ever go out of scope, so perhaps there really isn't a reason to automatically create a fresh copy upon insertion. If a pointer is passed to InsertElem(elem* ptr), then I guess we can take the STL approach of simply storing the ptr? I was worried that someone might call Insert(&local_obj), and then when the caller exits, local_obj is gone, and the container is holding a ptr to undefined data.. Where is my thinking mistaken, and when should a container positively copy inserted elements? – Erika Electra May 14 '12 at 06:47
@Cindeselia if you return the pointer as per your example, then the caller owns it and you should document it. You can make these semantics explicit by returning a `unique_ptr`. But I think returning by value here is by far the best option. – juanchopanza May 14 '12 at 06:48
The container creates a copy because I programmed it that way. But I admit I never thought about ownership, as I was never taught to. Embarrassingly, it was never addressed in texts and lectures where I learned. In a nutshell (or a resource, if you can recommend one), how do I make that determination if I'm implementing a generic container class with no knowledge of anything else around it? – Erika Electra May 14 '12 at 06:49
`Insert(&local_obj)` is an error on the programmer-side, and you can do little to defend against it (nor should you). If you simply store pointers, document that and then rely on users of your code to get it right - it is their responsibility. – Björn Pollex May 14 '12 at 06:49
You don't *determine* who owns an object, you *decide* it. The important thing is that all involved parties agree on who the owner is. – Björn Pollex May 14 '12 at 06:51
Okay, so I need to make a choice myself, document it, and then stay consistent throughout the class. I read Hurkyl's comment, and think that with a generic class, the STL approach of not claiming ownership seems safe...? Thank you for the insight! – Erika Electra May 14 '12 at 06:52
@juanchopanza - If I'm containing const char* however (if I can't use string, for whatever reason), it seems I have to return a pointer... unless you mean I should return an original copy? – Erika Electra May 14 '12 at 07:20
@Cindeselia oh i see, you are returning a pointer that is the beginning of a character string, not a pointer to a self contained object. Yes, that limits what you can do. If the original is heap allocated then you could just return that. I would just replace the `char` pointers by std::strings... – juanchopanza May 14 '12 at 07:41
Yes, Pop() is getting sticky because it blurs the line between caller ownership and container ownership. I can't seem to write it so that the container frees all the memory it allocates. I probably shouldn't support such a function, and limit the container's interface to memory it clearly owns, not letting it transfer ownership in confusing ways. – Erika Electra May 14 '12 at 10:01

Pop() etiquette

1 Answers1