Queue != Stack
Queue
behaviour is FIFO. Stack
behaviour is LIFO. Currently your datastructure is called Queue
, but it implements Stack
, has stack error messages etc. This is very confusing.
Magic Numbers
There are three main numbers you're using for manipulating your stack 5, 4 and -1. Out of context, it's not immediately obvious what they mean. Replacing them with constants would make the code easier to read.
Undefined behaviour
As has been hinted at by @Quuxplusone, this:
_myarr[5]={0};
Isn't doing what you're expecting it to. Rather than initializing all 5 of the elements in the array, which is what I suspect you expected, it is attempting to initialize the element with index 5. Since arrays are index from 0 and it only contains 5 elements, you're overflowing the array. This may or may not work, depending on how lucky you get.
Return Values
You've declared PUSH
,POP
,TOP
all returning type T
, however you're never returning anything. Depending on your compiler + switches this may or may not actually compile. PUSH
probably doesn't need to return the value it's added anyway...
Console Output
Console output is great for testing your code to see if it's doing what you're expecting, however writing it into your classes gives the console access to private data members. Try to avoid putting console interaction in your data structures. If you had written the console interactions outside of the class, you would have noticed that POP etc didn't return values a lot earlier in the cycle.
Error Conditions
If I pop from an empty stack, you write to the console, however the caller has no way to know that they haven't retrieved a value. Worse, if I push onto a full stack you simply throw the value away. You should notify the user that there has been a failure, possibly by throwing an exception.
Exit
"Any other button" isn't a great hint that you need to press 4 or above to get out of the application. Help your users, be explicit.
using namespace::std
Is generally frowned on, be explicit.
Indentation
Indentation matters, be consistent, it's a lot easier to read.
Putting it all together
I'm sure there's still room for improvement, however combining all of the above, the Stack would look a bit more like this:
#include<iostream>
#include<iomanip>
#include <exception>
template<class T>
class Stack {
private:
static const int stack_size = 5;
static const int stack_full = stack_size - 1;
static const int stack_empty = -1;
T arrays[stack_size] = {};
int i = stack_empty;
public:
void PUSH(T item)
{
if (i == stack_full)
{
throw std::runtime_error{ "Stack is full" };
}
i++;
arrays[i] = item;
}
T POP()
{
if (i == stack_empty)
{
throw std::runtime_error{ "Stack is empty" };
}
T &removed = arrays[i];
i--;
return removed;
}
T TOP()
{
if (i == stack_empty)
{
throw std::runtime_error{ "Stack is empty" };
}
return arrays[i];
}
};
int main()
{
Stack <int> Q;
int option, push;
do {
std::cout << "Select any of the following option" << std::endl;
std::cout << "1.Push\n2.Pop\n3.Top\n4.Exit" << std::endl;
std::cin >> option;
try {
switch (option)
{
case 1:
std::cout << "Which element to push" << std::endl;
std::cin >> push;
Q.PUSH(push);
break;
case 2:
{
int removed = Q.POP();
std::cout << removed << " has been popped" << std::endl;
break;
}
case 3:
{
int top = Q.TOP();
std::cout << "Element at top of stack is " << top << std::endl;
break;
}
case 4:
break;
default:
std::cout << "Error!" << std::endl;
break;
}
}
catch (std::runtime_error error) {
std::cout << error.what() << std::endl;
}
} while (option < 4);
}
PUSH
,POP
andTOP
say that they return aT
, but they actually do not. You should consider turning on warnings and "treat warnings as errors" to make the compiler check these things for you. \$\endgroup\$