C++: calling member functions within constructor?

Question

The following code raises a runtime error:

#include <iostream>
#include <iterator>
#include <ext/slist>

class IntList :  public __gnu_cxx::slist<int> {
public:
    IntList() { tail_ = begin(); } // seems that there is a problem here
    void append(const int node) { tail_ = insert_after(tail_, node); }

private:
    iterator tail_;
};

int main() {
    IntList list;

    list.append(1);
    list.append(2);
    list.append(3);

    for (IntList::iterator i = list.begin(); i != list.end(); ++i) {
        std::cout << *i << " ";
    }

    return 0;
}

Seems that the problem is in the constructor IntList(). Is it because it calls the member function begin()?

Why are you typedefing `iterator` to itself? And, if this `slist` is based off of the STL `slist` then it doesn't have a virtual destructor, so you probably shouldn't derive from it publicly (or at all). — James McNellis, May 10 '10 at 23:24
For me the problem is the append(). Have you compiled with the -g flag. Also it probably help to add print statements to see where it crashes — Martin York, May 10 '10 at 23:32
@James and poerboy. Technically yes. But not really relevant to the question. — Martin York, May 10 '10 at 23:33
@James McNellis: Its very standard to typedef types from inherited classes like that. Look at the definition of nearly every container in the STL. — Martin York, May 10 '10 at 23:41
@Martin: It was unusual in this case, because `slist::iterator` is a public typedef, so it was wholly unnecessary. If a public base has a public typedef, there's no reason to redeclare it in the derived class. As for my comment not being relevant to the question, that's why I posted it as a comment and not an answer. Using comments for questions or advice (e.g., don't derive from a class that doesn't have a virtual destructor) is common practice. — James McNellis, May 10 '10 at 23:48

Maciej Hehl · Accepted Answer · 2010-05-10T23:33:52.427

2

Looks like You are inserting after end();

In the body of the constructor

IntList() { tail_ = begin(); }

the base class is constructed and it's members can be called, but for an empty list, it should return end();

edited May 10 '10 at 23:33

answered May 10 '10 at 23:25

Maciej Hehl

7,895
1
22
23

This looks right; the iterator passed to `insert_after()` must be dereferenceable, and `end()` is not. – James McNellis May 10 '10 at 23:31
So, I want to add the private member tail_ to track the tail of the list so that I can append elements faster. How can I do this? – powerboy May 10 '10 at 23:33
You shouldn't derive from `slist`, to begin with. But to solve that particular problem try calling `insert()` instead. – wilhelmtell May 10 '10 at 23:36
Thx WilhelmTell! Maybe I should use slist as the underlying container of my list, the same way as how is based on – powerboy May 10 '10 at 23:55
3

@powerboy yes, that makes more sense. either bring an `slist` object into `IntList` as a data-member, or derive privately from `slist`. The problem is there only when clients try to create a polymorphic object of `IntList`, like so: `slist* l = new IntList();` and sometime later `delete l;`. When that happens the `slist` part of `IntList` will clean up properly, but the `IntList` won't: it will result in undefined behaviour. If you don't derive publicly from `slist` then users simply can't make a polymorophic object of the concrete `IntList` type, so it's safe. – wilhelmtell May 11 '10 at 00:05
@powerboy if you derive privately from `slist` then remember that inside your class you still have access to `slist`. that means that any member (or friend) of your class can still create and delete a polymorphic type that interfaces on `slist`. In other words, _you_ can still shoot yourself in the foot. For this reason it's a bad idea to derive in any way from a class that has no virtual destructor. But yes, you'd still limit the problem to you shooting yourself in the foot, rather than anyone at all. – wilhelmtell May 11 '10 at 00:20

wilhelmtell · Answer 2 · 2010-05-10T23:57:18.587

2

Your problem is not in the constructor but in the first call to append(). Because your list is empty begin() equals end(). end() is a valid iterator, but the one after that isn't. To solve that particular problem try calling insert() instead.

That said, a fast peek at <ext/slist> confirms that the destructor of slist isn't virtual, which means slist wasn't intended to be derived from.

edited May 10 '10 at 23:57

answered May 10 '10 at 23:33

wilhelmtell

57,473
20
96
131

Martin York · Answer 3 · 2010-05-10T23:43:17.683

The documentation for slist<> indicates that the iterator provided to insert_after must be dereferenceable.

Since your list is empty the begin() returns end() and is thus not dereferenceable.

See documentation here:

http://www.sgi.com/tech/stl/Slist.html

iterator insert_after(iterator pos, const value_type& x)

pos must be a dereferenceable iterator in *this. (That is, pos may not be end().) Inserts a copy of x immediately following pos. The return value is an iterator that points to the new element. Complexity: constant time.

score 0 · Answer 4 · answered May 10 '10 at 23:23

0

.begin() is invalid if there are no elements in your collection.

answered May 10 '10 at 23:23

Chris K

11,996
7
37
65

`begin()` is always valid on a container. If there are no elements in the container, then `begin() == end()` (assuming this GNU extension slist is an implementation of the STL slist, of course). – James McNellis May 10 '10 at 23:25
Learn something new everyday, I guess. :-) – Chris K May 10 '10 at 23:29

C++: calling member functions within constructor?

4 Answers4

Linked