[stack][list]I can't understand my mistake

Question

Help please, I can't understand where is mistake, after compilation, program has nothing to show. Need to create 2 stacks, one of them will contain random numbers, the other will contain only those numbers from first stack, which greater than average of all numbers in first stack

#include "stdafx.h"
#include <iostream>
#include <ctime>

using namespace std;

struct stack //our stack
{
    int num;
    stack* ptr = NULL;
};

void addtostack(int &n, stack* p) //add element to stack
{
    stack* newstack = new stack;
    newstack->num = n;
    newstack->ptr = p;
    p = newstack;
}

void showstack(stack* p) //show stack
{
    stack* current = p;
    while (current->ptr)
    {
        cout << current->num << endl;
        current = current->ptr;
    }
}

int main()
{
    srand(time(NULL));
    const int SIZE = 5;
    stack* first = new stack;
    int rnd;
    double average = 0;
    for (int i = 0; i < SIZE; i++)
    {
        rnd = -50 + rand() % 101;
        addtostack(rnd, first);
        average += rnd;
    }
    average /= SIZE;
    showstack(first);
    system("pause > nul");
}

And what's the problem you're experiencing? (Error message, behaviour, etc) — Steve, Dec 07 '17 at 16:17
In `newstack->ptr = p;` you're adding `first` to the ptr of the new stack, but then you're performing your show starting from first. Do you mean to be doing `p->ptr = newstack`, which would then mean you don't need to override p as you are? — LordWilmore, Dec 07 '17 at 16:20
I am sorry to inform you that you are not being taught properly. This is horrible code. It is not horrible because you are a beginner and don't know better, it is horrible because you are being taught to write horrible code. If you want to learn C++ [buy a book](https://stackoverflow.com/a/388282/3484570) and ignore everything they are teaching you. — nwp, Dec 07 '17 at 16:20
Have you used the debugger to make sure each line is doing what you expect? Is it possible to write unit tests to test your individual methods? We do it all the time for C#, Java & JavaScript. — Ian Robertson, Dec 07 '17 at 16:30
Of course, and didn't undestand where is problem. Pointer errors are difficult to catch, that's why i asked question here, and the answer below is correct, and my code show numbers in console. I don't understand why my code is terrible, I read Robert Lafore now... — Javid, Dec 07 '17 at 16:36
Welcome to Stack Overflow! Please [edit] your question to show us what kind of debugging you've done. I expect you to have run your [mcve] within Valgrind or a similar checker, and to have investigated with a debugger such as GDB, for example. Ensure you've enabled a full set of compiler warnings, too. What did the tools tell you, and what information are they missing? And read Eric Lippert's [How to debug small programs](https://ericlippert.com/2014/03/05/how-to-debug-small-programs/). — Toby Speight, Dec 07 '17 at 17:29

score 1 · Accepted Answer · answered Dec 07 '17 at 16:22

The line

p = newstack;

does not do what you hope it will do.

It changes a local copy of the variable p. The pointer in main still points to the only object that it was initialized with.

You can solve it couple of ways:

Pass p by reference. You don't need to pass n by reference though.

void addtostack(int n, stack*& p) //add element to stack
{
    stack* newstack = new stack;
    newstack->num = n;
    newstack->ptr = p;
    p = newstack;
}

Return the new pointer to the calling function.

stack* addtostack(int n, stack* p) //add element to stack
{
    stack* newstack = new stack;
    newstack->num = n;
    newstack->ptr = p;
    return newstack;
}

and then change main to:

for (int i = 0; i < SIZE; i++)
{
    rnd = -50 + rand() % 101;
    first = addtostack(rnd, first);
    average += rnd;
}

[stack][list]I can't understand my mistake

1 Answers1