Using top with empty stack

Question

So i have a custom stack class and a top function. Im checking if the top is empty but am having some trouble returning a value without error.

Error: "Cannot convert int to &int"

    dataType &top()
{
    try
    {               
        if (stackData.checkEmpty()== false)
        {
            throw stackData.size();
        }
    }
    catch(...)
    {
        cout << "Stack size is zero. Can not retrieve top " << endl;
        return stackData.size(); //***Where the problem is***
    }

    return stackData.back();
}

int size( ) const
{
    return Size;
}

`size()` needs to return an `int &` for that to work. Of course that's not a particularly good idea. I don't see why you return the size in the first place, or throw an exception just to catch it two lines later. — chris, Sep 28 '13 at 20:25
@Rob you should probably just raise an exception and be done with it. — juanchopanza, Sep 28 '13 at 20:28
With no return i get erroneous return values to the output. Ex: " Stack is empty!" Stack top: -3395727293" — Rob, Sep 28 '13 at 20:29
@Rob, No return is undefined behaviour. Just don't handle the exception inside of the function. — chris, Sep 28 '13 at 20:30

score 2 · Answer 1 · edited May 23 '17 at 12:03

2

The problem is in your size method. You must be returning an rvalue or a literal. See this post for the same problem in a different setting.

Evaluate whether you really need to return a reference, since it is meaningless to have references to the top element when the stack is empty. It may be better to follow other's suggestions and throw an exception when top is called on an empty stack, returning the size of the stack will make virtually impossible to distinguish the empty case (that'd return zero) from when a zero integer is stored in the stack.

As a side note, for brevity, is it better to write:

if (!stackData.checkEmpty())

than

if (stackData.checkEmpty()== false)

since checkEmpty() is already returning a boolean. This is just style.

edited May 23 '17 at 12:03

Community

1
1

answered Sep 28 '13 at 20:28

papirrin

2,004
22
30

2

It's `size()` causing the problem. And it does make sense to return a reference (`std::vector` does for `front` and `back`), but calling it on an empty stack is dumb anyway, so why should you have to change the return type because a reference doesn't make sense for a case that doesn't make sense to begin with? I do agree on the style note, though. – chris Sep 28 '13 at 20:33
@chris, yes, I meant that is meaningless to attempt to retrieve a reference on an empty stack. I assumed the asker does not want to have an exception thrown, since he has the handler in the call. Hence, he must rethink his approach to either do as you say or remove the reference. – papirrin Sep 28 '13 at 20:35
Well, if the exception really is a big problem, something like `boost::optional` could be used. Handling it like that, the exception shouldn't even be thrown in the first place. It bloats the code and makes no sense. – chris Sep 28 '13 at 20:36
Im looking to simply return to output that the stack is empty. However i ouput that but an erroneous value still comes with it. All im trying to eliminate is the erroneous value – Rob Sep 28 '13 at 20:45
See the edit, your size method is returning a literal or a rvalue. Do as @cris said to fix it (throw an exception). – papirrin Sep 28 '13 at 20:48
@Rob well, what is the magic number you intend to return to signify that the stack is empty? You could just return a reference to a static variable with that value. – juanchopanza Sep 28 '13 at 20:48

score 0 · Answer 2 · answered Sep 28 '13 at 20:35

0

stackData.checkEmpty() reads like it returns true if empty but you test for false and throw which seems the wrong way round.

Your function top() has a signature to return dataType& but on this condition you return an int... you can't do that.

answered Sep 28 '13 at 20:35

Oakdale

138
1
7

Not true. Check empty: if (size <=0) return false; else return true – Rob Sep 28 '13 at 20:37
@Rob - implementation-wise it works, but Oakdale is correct in that the naming here is misleading. – Asaf Sep 28 '13 at 21:15

Using top with empty stack

2 Answers2