On Mon, 2004-04-05 at 14:13, Alex Karasulu wrote:
> You really don't know how many times the empty stack is going to have pop or
> peek called by a client. So this really is not a situation that we know is
> only going happen once and a while. "Cheaper" comes from the fact that it
> costs let to check if the stack is empty than it does to create an exception
> object, fill its stack trace, and hurl it out the door to have it caught
> again.
>
> However it certainly is a good point that the conditional test for empty
> will always occur. Making the change really is not a big deal I thought I'd
> just point it out. We can just leave it as is.
I would expect that anywhere in Digester, popping an empty object stack
is (or should be) regarded as an error which immediately terminates all
processing. I don't know why this method was ever coded to return null
when an empty stack is popped. So the situation you describe, in which
an Exception object is created and filled in, should never happen in a
successful parse, and only once in an unsuccessful parse.
You will see in CVS that Robert's new "named stack" pop method does
throw an EmptyStackException when an empty stack is popped, and I think
that is the correct behaviour. If we do a digester 2.0, I would
recommend that the pop() method throw an exception in this case - unless
I've missed a good reason for the null to be returned. I can't think of
one, though.
> > Of course the current exception-based approach is thread-safe, while an
> > if-based approach is not, though that is not an issue here.
>
> Please excuse me for being dense but how is throwing the exception a thread
> safe situation? Before the exception is thrown does the stack not check the
> same variable to see if it is empty or not?
Yes, but if the stack implementation is a thread-safe stack, then the
exception-based approach will also be thread-safe. However the if-based
approach is *always* vulnerable to a race condition if some other thread
can change the stack size between the if and the pop:
if (stack.isEmpty()) {
return null;
} else {
// right here some other thread could empty the stack, causing
// the following pop to throw an exception despite our
// check above
return stack.pop();
}
Cheers,
Simon
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
