Linked List, Newbie question

[Georg Madler]

Hi, I've got the following newbie-problem:
I try to create a linked list existing out of 5 elements for example.
To later process the list I try to create a pointer pointing on the
first element in the list. But as soon as I add a new element to the
list this pointer changes to the new element, too. How do I store a
pointer pointing on the first element of the list? Thanks...

class Point {
public:
Point New();
void SetCounter(int count);
private:
Point *next;
int counter;
};

Point Point::New() {
this->next = new Point;
return *this->next;
}

void Point::SetCounter(int count) {
this->counter = count;
}

int main()
{
Point *point = new Point;
Point *start;

//Save pointer on the first element of the list
start = point;

for(int i=1;i<=5;i++) {
point->SetCounter(0);
*point = point->New();
}

return EXIT_SUCCESS;
}

 

[Howard]

Hi, I've got the following newbie-problem:
I try to create a linked list existing out of 5 elements for example.

Actually, your code creates 6 objects. One it creates initially, then 5
more in the loop.

To later process the list I try to create a pointer pointing on the
first element in the list. But as soon as I add a new element to the
list this pointer changes to the new element, too. How do I store a
pointer pointing on the first element of the list? Thanks...

class Point {
public:
Point New();
void SetCounter(int count);
private:
Point *next;
int counter;
};

Point Point::New() {
this->next = new Point;
return *this->next;
}

That function creates a new Point object, and returns a COPY of it. If you
want a pointer to the new object, return a Point*, and just have

return next;

(There's no reason to add "this->" to anything in most cases, except in
certain cases when dealing with templates. It's implied already that you're
talking about "this" object.)

You might also want to consider calling it AppendNew, since it's really
doing two jobs: creating a new object and adding that to the list.

Not the best design, by the way. You should probably create a new object in
main (or somewhere outside the class itself), and then pass a copy of that
pointer to an Append function of this class. That gives you much better
control over the pointers, and allows better error handling.

void Point::SetCounter(int count) {
this->counter = count;
}

Instead of calling this function to set counter to 0, you REALLY should have
a constructor for the Point object. Most likely, you'd want it to also set
the next member to NULL, so that you're sure it's always going to be NULL
unless you explicitly create a new object via your New call. That's
important, because of what happens when thes objects are destroyed!

int main()
{
Point *point = new Point;
Point *start;

//Save pointer on the first element of the list
start = point;

What's this for? You never use it. (ALthough, see later for where you
*might* want to use it...)

for(int i=1;i<=5;i++) {

You should get into the habit of using ++i in 'for' loops instead of i++.
It doesn't matter here, but there are cases where it does matter, and it's
simply better to develop that habit from the start.

point->SetCounter(0);
*point = point->New();
}

return EXIT_SUCCESS;
}

Now what? You've got a memory leak here. You've created 6 objects, and
none of them get destroyed.

You should have a destructor for this class. Any time you manage pointers
in your class, you really need a constructor, destructor, (and a copy
constructor as well). Look up the "Rule of Three".

You'll need to think what you want your destructor to do. If it calls
'delete' on the 'next' member, then all you have to do is delete the 'start'
object, and you're done. That's because the first object will delete the
second, which will then delete the third, and so on. BUT!...you will
definitely need a constructor that sets next to NULL, so that you don't get
undefined behavior (such as a crash!) when deleting. That next member must
either be valid or 0, and without a constructor you may end up with it being
some random value.


-Howard

 

[Georg Madler]

Thanks Howard!
With your help I got my problems sorted out...

Kind regards,
Georg

 

[Edwin Fine, Fine Computer Consultants]

It almost, but not quite, goes without saying that in "real life", you
should usually use the STL for containers such as linked lists
(stl::list). Please don't roll your own, except as an exercise or when
you are no longer a newbie and understand all the tradeoffs involved.