Implementation of Stack Data structure

Question

https://code.sololearn.com/cvIMhGlSHHK8/?ref=app

Answer 1

Well, since you asked so nicely, I'll give you some criticism.

Don't use global variables, use a class and move all variables/functions into the class. ( or just move the variables into a struct in the case of C, and then take a pointer to that struct for each function to access the variables )

Use the same naming convention for all functions.
isEmpty and push_var use camelCase and snake_case respectively.
Pick one and stick with that so either is_empty and push_var or isEmpty and pushVar.

if (top == -1)
{
return true;
}
else
{
return false;
}

Use 'return top == -1;' instead. It's much shorter and more readable.

In functions like push_var, pop_var, peek and display, they don't return after the isEmpty check so you run the chance of accessing the array at an index of -1 which is undefined behavior. Either return or throw something. ( and then remove the cout and handle that elsewhere with the catch )

You're shadowing the 'size' variable in main. The global size just happens to be initialized to 0 because it's global so isFull doesn't function properly.

Don't use #define to define a constant variable ( that's C ), use constexpr in modern C++. Or a const variable if that is not available.

Lastly, try not to use 'using namespace std;' in real code. It's fine for practice though. In this case you're getting an error because std::size exists.

Answer 2

This place is not for advertising your code, it's only for programming related discussion. Have a look at the forum guidelines
https://www.sololearn.com/discuss/1316935/?ref=app

Answer 3

Implementation of stack using linked list

https://code.sololearn.com/cl9wCnD8NKms/?ref=app

Answer 4

That looks much better. However there are still some issues.
The big ones are the global variables.
You really don't want them or need them.

I'm assuming C here instead of real C++.

You can remove the #define size 100 because you're not using it anyway. Use 'const int Size = 100;' instead if you do need it ( note the capitalization, it's a good idea for constants ) if you really want to use a #define make it all caps like '#define SIZE 100' to make it obvious that you're using a macro instead of, say, a variable.

int value; can be moved into main, it has no reason to be global.

Currently, your functions only operate on 1 instance of struct node. What if you needed more than 1 stack? Your code doesn't support that.

Move struct node* top = NULL; to main, then modify the functions to take a pointer to the node as the first argument. Take double pointers whenever you need to modify the actual pointer itself in a function. ( whenever you need to assign something to top itself )

void push( int value ) becomes void push( struct node** top, int value )
void pop() becomes void pop( struct node** top )
but something like int peek() becomes int peek( struct node* top )

Then add a pointer to whatever used top before in the functions that now take a double pointer.
For example 'newnode->link = top;' becomes 'newnode->link = *top;'

Then call those functions with push( &top, value ); ( add & for the functions that require a double pointer ) and peek( top );

Another issue is that you're code will crash if the user enters a 3.
That's because peek doesn't return or throw if top == NULL. Resulting in a NULL dereference in the line after.

The rest looks fine to me. :)

Answer 5

Dennis
It works fine

Answer 6

Yes, it does work, and good work with that.
I'm merely providing information on how you could improve it for next time because code can always be improved.

Answer 7

Dennis
Thank you for suggestions i will improve code