3

I would like to ask 2 question about this code. Where I just try to simulate a stack.

Stack.h

 #pragma once

namespace stackandqueue {

    class Stack
    {
    private:
        int index;
        int *stackdata;

    public:     
        Stack();    
        ~Stack();
        void push(int val);
        int pop();
        int top();
        bool isEmpty();
    };

}

Stack.cpp

#include "stdafx.h"
#include "Stack.h"

namespace stackandqueue {

        Stack::Stack() : index{ 0 }
        {
            stackdata = new int[index];
        }
        Stack::~Stack()
        {
            delete[] &stackdata;
        }
        void Stack::push(int val) {
            stackdata[index] = val;
            index++;
        }
        int Stack::pop() {
            int val = stackdata[index];
            index--;
            return val;
        }
        int Stack::top() {
            return stackdata[index];
        }
        bool Stack::isEmpty() {
            return index == 0;
        }
}

Meaning is to let me create

 Stack stack;

And then it initilizes a dynamic array with 0 as first index and that let me push, pop, top values.

First question: Why am I having unresolved symbols for method definitions?

Second question: About 'stackdata', you find is the right way if I want to declare an "array" with dynamic size for this behaviour?

I'm open for improvements and best practices. Im used to programming languagesbut I've never delved into c ++ and I don't want to have bad practices. So you see I am taking it from the begining.

Thanks.


I post solution reached with your help that maybe helps someone.

class Stack
    {
    private:
        int index;
        int* stackdata;

    public:     
        Stack(int size);    
        ~Stack();
        void push(int val);
        int pop();
        int top();
        bool isEmpty();
    };

    Stack::Stack(int size) 
        : index {0}, stackdata{new int[size]} 
        {
        }
        Stack::~Stack()
        {
            delete[] stackdata;
        }
        void Stack::push(int val) {
            stackdata[index] = val;
            index++;
        }
        int Stack::pop() {
            index--;
            return stackdata[index];
        }
        int Stack::top() {
            return stackdata[index-1];
        }
        bool Stack::isEmpty() {
            return index == 0;
        }

1 Answers1

5

There are several problems with this.

  1. An array, dynamically allocated or otherwise, is not a stack/queue/vector. You're creating literally 0 ints. All of your element accesses after that have undefined behaviour. You need your array to grow, i.e. be a vector, e.g. std::vector.

  2. delete[] &stackdata has the wrong level of indirection. You meant delete[] stackdata. You were trying to delete the pointer which was not dynamically allocated.

  3. You're missing copy/move constructors and copy/move assignment operators so as soon as you transport a Stack anywhere it will explode. (The original instance will do a delete[] on the same pointer that the copied/moved instances will!) Read about the rule of three/five/zero.

Other than that, it looks like a stack.

The problem you don't have here is an undefined reference, which is funny because that's the only one you asked about. :) If you do indeed have such a thing, it's likely a problem with your build system (failing to compile that source file), which we cannot see.

Lightness Races in Orbit
  • 378,754
  • 76
  • 643
  • 1,055
  • "other than that".. :) I understand. So the point is I have not a way to say "Ok, I point you the first address and the size of the data (int) and from here you grow up and assign memory dynamically and reallocate if needed"'? I would like evade std::vector for now. Must I define a known-sized array yes or yes? – FooBarExtension Mar 05 '19 at 20:50
  • @FooBarExtension Yes. You would have to create a class (just the same as vector!) that reallocates a bigger block of memory, copies the old contents into it, then de-allocates the old, smaller block of memory. Optionally shrink the buffer as needed too (though peeps don't tend to do this; vector doesn't). `new[]` means you can provide runtime bounds, but it doesn't mean infinite/changeable bounds; the memory you ask for is the memory you get. – Lightness Races in Orbit Mar 05 '19 at 20:51
  • 1
    Got it. I heard "Don't invent what is invented yet". I will try using std::vector. Or I will require size in constructor to make it dynamic. I undestand now. Thank you very much. :) – FooBarExtension Mar 05 '19 at 20:54