Wanted some feedback on stack implementation C++

Question

I just wanted some feedback on my C++ implementation of Stack. I know this is somewhat messy but I am trying to improve the way I use classes. I am pretty sure that I am not using templates properly.

Any suggestions are welcome!

#include <iostream>
#include </root/Dev/Stack.h>
#include<string>

using namespace std;

template<class T>
Stack<T>::Stack(T &size)
{
    capacity = size;

    if(size==0)
    {
        cout<<"Capacity can't be zero!"<<endl;
        stack = 0;
    }
    else
    {
        stack = new T[capacity];
    }

    count = -1;
}

template<class T>
void Stack<T>::Insert(T &data)
{
    if (count == (capacity-1))
    {
        cout<<"Stack is full";
        return;
    }
    else
    {
        count++;
        stack[count] = data;
    }
}

template<class T>
void Stack<T>::Display()

{
    cout << "Printing out values:" << endl;
    for (int i=0; i<count; i++)
        cout << stack[i] << endl;
}

int main()
{
    Stack<int> S1(5);

    S1.Insert(10);
    S1.Insert(22);
    S1.Insert(5522);
    S1.Display();

    Stack<string> S2(6);
    S2.Insert("hi");
    S2.Insert("Savvy");
    S2.Insert("How are you?");
    S2.Display();

    return 0;
  }

The header file:

#include<iostream>

using namespace std;

template<class T>
class Stack {

public:
    int capacity;
    int count;
    T *stack;
    void Insert(T &num);
    void Display();

    Stack(T &value);
  };

@CaptainObvlious Please vote to close as "too broad". Voting to close because a question belongs on another site is not a valid close reason. — Ethan Bierlein, Sep 23 '15 at 01:16
@EthanBierlein It sure is a valid reason. It's simply off topic here, but is on topic elsewhere. — Kuba hasn't forgotten Monica, Sep 23 '15 at 22:55
@KubaOber No, it *is not a valid close reason.* If there's a close reason which is better than it, and already built-in, then it should be used. — Ethan Bierlein, Sep 23 '15 at 22:56
@EthanBierlein I don't think it's too broad. It's a decent code review post, assuming that the code works to begin with. — Kuba hasn't forgotten Monica, Sep 23 '15 at 22:57
According to the Help Center, the main opposition to this question would be _"a practical, answerable problem that is unique to software development"_ and while it does hit the 2nd mark, it does not quite hit the first, hence "too broad" would be the most appropriate close reason. Nevertheless, this question _could_ be good on Code Review, with some work. It would need to state in the non-code portion and the title what the code _does_, and not what kind of answer is desired. — Phrancis, Sep 23 '15 at 23:01

score 0 · Answer 1 · edited May 23 '17 at 12:22

I'm pretty sure that this should not compile. You've not declared the template in the source file (it's a weird requirement, see here: Why can templates only be implemented in the header file?). That can be resolved by adding a template class Stack<int>; and a template class Stack<string>; before int main.

Otherwise, this should work very well if you add a destructor (whose body would basically be delete[] stack;) and added a way to remove from the stack (it is usual functionality).

Also, consider making T* stack private for encapsulation.

Finally, in insert, consider throwing an exception instead of returning after printing. It would be better for use in a big project.

Other than the template declaration issue, your use of templates is perfectly fine.

I tried doing that and it still failed. I have the implementation in my header now. Also, people on codereview tell me that code that doesn't work is off-topic :( — Savitra Sapre, Sep 23 '15 at 22:08

score 0 · Answer 2 · edited Sep 23 '15 at 01:43

Your implementation about Stack class template will not work well, it will fail during compilation. Why?

The declaration and implementation of any class template cannot be separated in .cpp and .h 2 files, otherwise your compilation will fail.

Because template class Stack {...} class T is arbitrary. As you know, C++ is a kind of static language, as the result that the compiler will need to know the exact memory size which should be allocated for every data (or object) type during compiling process.

But class template T is arbitrary, you don't know its exact memory size which should be assigned before invoking. So compiler cannot initialize the class template object.

And there is a common sense that all compilers, wherever it comes from, Microsoft, ANSI, ISO, Google or GCC, cannot support to compile the separated template code. The declaration and implementation of all templates (class template and function template) must be written in the header file.

If you don't believe, you can go to see the source code of STL, all STLs were defined and implemented in the header file.

So, you should rectify your traditional programming principle slightly, combine the declaration and implementation of template into one header file.

I did. but I still don't know why it doesn't work. Shows me a lot of linker errors. — Savitra Sapre, Sep 23 '15 at 22:07

Wanted some feedback on stack implementation C++

2 Answers2