bug in stack

[asit]

what's wrong in the following code ??

#include <iostream>
#include <string>

using namespace std;

struct node
{
int data;
node *prev;
};

node* push(node * st, int data)
{
if(st == NULL)
{
st = new node;
st->prev = NULL;
st->data = data;
}
else
{
node* t = new node;
t->prev = st;
t->data = data;
st = t;
}
return st;
}

 

[Alain Ketterlin]

what's wrong in the following code ??

struct node
{
int data;
node *prev;
};

node* push(node * st, int data)
{
if(st == NULL)
{
st = new node;
st->prev = NULL;
st->data = data;
}
else
{
node* t = new node;
t->prev = st;
t->data = data;
st = t;
}
return st;
}

Do you really think both cases are different? Is it really relevant to
distinguish a null incoming stack and a non-null stack?

(By the way, I would advise to not use function arguments as local
variables. You'll save nothing, the compiler is anyway much better at
optimizing than you are. Use as many local variables as you want/need,
give them significant names, and avoid reassigning different values to
the same variable through the course of the function. If you apply this
rule to the code above, you'll see why I ask the questions above.)

Finally, how do you call push()? Your version returns the "new stack",
make sure you use that value.

I guess you haven't learnt constructors yet. If you have, try to think
about how you could use them here. If you have not, forget about them
for now.

-- Alain.

 

[Michael Doubez]

Why do you think there is something wrong with this code?




not needed


not needed




not needed





duplicate code, the else branch has exactly the same functionality.


Other than lots of unneeded lines and missing specifications about the
purpose of the struct and the function, I do not see nothing wrong with
this code.

He is assigning the function parameter value 'st'. It shoukld either
be a pointer-pointer or a pointer-reference.

A coworker of mine thinks all parameter values should be const; I
disagree for readability reasons but it could be a interesting
compiler option.

[snip]

 

[Paul]

what's wrong in the following code ??

#include <iostream>
#include <string>

using namespace std;

struct node
{
int data;
node *prev;
};

node* push(node * st, int data)
{
if(st == NULL)
{
st = new node;
st->prev = NULL;
st->data = data;
}
else
{
node* t = new node;
t->prev = st;
t->data = data;
st = t;
}
return st;
}

If you pass another stack as a node any original stack nodes are
unreachable.
The problem is that the data-node and the stack are both the same object.

HTH

 

[Martijn van Buul]

* Christian Hackl:

asit ha scritto:


Most importantly, the fact you don't use a standard C++ container class,
such as std::stack (if you really want a stack).

Is that so? I think it's a bit of a condescending remark. There are a
lot of reasons why you'd want to implement a stack (or a list, or a
set) yourself - including a didactic one:

You won't appreciate the value of a calculator if you never did any number
crunching by hand.

 

[Paul]

asit ha scritto:


Most importantly, the fact you don't use a standard C++ container class,
such as std::stack (if you really want a stack).

I agree with Martin , I find this comment a bit annoying..

How did C++ programmers even manage pre-STL?

 

[Paul]

What utter nonsense. 'push' returns a pointer to the new top of the stack
*passed to it*; it doesn't affect any other pointers or stacks; you do not
know what the OP is doing with the return value of 'push'.

Utter nonsense is your deparment.

You probably don't even understand so I will explain:
Node* st1, st2;
st2->prev = push(push(push(s1->prev,0),0),0);
st1 = push(push(st2->prev,0),0);

 

[Martijn van Buul]

* Leigh Johnston:

I need to hammer in a nail. I have a hammer in my toolbox but I prefer
to bash nails in with this heavy rock instead. The moral of the story:
don't make work for yourself.

But many parts of STL (and even moreso: boost!) aren't really compareable
with a hammer. They're more like a powerdrill. Useful, convenient to have
around, and there's tons of people who think you need nothing else.

 

[Martijn van Buul]

* Paul:

You probably don't even understand so I will explain:
Node* st1, st2;
st2->prev = push(push(push(s1->prev,0),0),0);

Segmentation fault, core dumped.

 

[Paul]

* Paul:


Segmentation fault, core dumped.

It's ok its only for Leigh.

 

[Paul]

Any why on earth would you write that code (ignoring the fact that it is
UB)? What exactly are you trying to achieve? Like I said: "utter
nonsense".

Node* st1 = push(push(push(0, 0), 0), 0);
st1 = push(push(push(st1, 0), 0), 0);

I see no problems here (nothing unreachable).

This code proves that.......you are an idiot!