-
Notifications
You must be signed in to change notification settings - Fork 17
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[FEAT] first solution for stack problem in cpp #52
base: main
Are you sure you want to change the base?
[FEAT] first solution for stack problem in cpp #52
Conversation
I have refactored the code according to Gabriele's advices :) |
int x = 0; | ||
if(isEmpty()) { | ||
cout << "Stack is Empty." << endl; | ||
return -1; //we have to reurn some value couse of the int function |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in cpp the int value is also negative from −2,147,483,648
to 2,147,483,647
it means that -1
is a possible result.
So this code isn't correct, you need to find another way
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I've tried to implement a workaround to manage this issue. Thanks!
} | ||
|
||
bool isEmpty() { | ||
if(myStack.size() == 0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there should be a way to use one line code here something like: return myStack.size() == 0
or similar
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok
} | ||
|
||
void display() { | ||
if (isEmpty()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can remove:
if (isEmpty())
cout << "|None";
when you display values, you don't need to specify that it is empty
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok
int x = 0; | ||
if(isEmpty()) { | ||
cout << "Stack is Empty. The values in stack are: "; | ||
return 0; //we have to return some value couse of the int function |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
0
is part of the int
so it is a valid value.
You need to find another way
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I’ve tried to find a workaround using different cout <<
but, as you suggest in the next comment, I think that the problem is here: I need to remove all the cout <<
from my class
return myStack.at(myStack.size() - 1); | ||
} | ||
else | ||
cout << "Stack is Empty. The values in stack are: "; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you have to remove all the cout <<
in your class.
The output print is very slow and has nothing to do with the class.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok I’ll work on it. Thanks
… and improve readability of CLI interface
My first attempt for the stack problem in cpp :)