We don't use event objects in Pivot - we pass the event information
directly to the event handler method as arguments (it saves the heap
allocation and makes the listener easier to understand). So the method
is already consistent with Pivot conventions.
I will make sure the Javadoc for the Application class is clear.
Thanks for your feedback.
On Jun 24, 2009, at 9:23 AM, Christopher Brind wrote:
In that case you should pass in an event object and do something
with the
event:
public void shutdown(ShutdownEvent event) {
if (someReasonToNotShutdown()) {
// don't want to shutdown...
event.cancel();
}
}
But then that becomes overly verbose, IMHO. I like the conciseness
of the
current design and wouldn't actually advocate creating an Event
object just
for this.
So long as it is well documented in the Javadocs, it should be fine
whatever
the final decision.
Cheers,
Chris
2009/6/24 Greg Brown <[email protected]>
It's more along the lines of consuming an event. "shutdown()" is a
notification. Returning true means that you rejected the
notification (like
consuming an event).
On Jun 24, 2009, at 9:13 AM, Christopher Brind wrote:
I'm not sure I agree... by calling shutdown, which returns a boolean,
you're
asking the question 'should the app shutdown?'
shutdown ? yes : no
Thus logically, you wouldn't answer "no" to the question
"shutdown?" if
you
wanted to actually shutdown.
Cheers,
Chris
2009/6/24 Greg Brown <[email protected]>
The enums are potentially just as confusing. Does continue mean
"continue
execution" or "continue shutdown"?
The shutdown() method is only meant to be invoked by the
application
context. Maybe it should be protected so that only subclasses and
the
org.apache.pivot.wtk package could see it. However, Application
is an
interface so that's not possible.
The conversation between the application context and the
application
might
go like this:
App context: "It is time to shut down. You can say no if you
want." ->
shutdown(true)
Application: "I don't want to shut down right now." -> return true
(cancels shutdown)
or this:
App context: "It is time to shut down. You have no choice." ->
shutdown(false)
Application: "OK." -> return false (value doesn't matter since
shutdown
is
not optional)
I don't think there's anything terribly confusing about either of
these
scenarios, or the return values.
On Jun 24, 2009, at 8:56 AM, Christopher Brind wrote:
Generally you should avoid double negative testing which I think
is what
would happen here...
*Scenario 1:*
boolean shouldShutdown = shutdown();
if (shouldShutdown) {
System.exit(0);
}
*Scenario 2:*
boolean dontShutdown = shutdown();
if (!dontShutdown) { // double negative
System.exit(0)
}
I think Scenario 1 is easier to understand.
Alternatively why not return an enum?
public enum ShutdownResponse { CONTINUE, ABORT };
And change the signature to:
public ShutdownResponse shutdown();
Cheers,
Chris
2009/6/24 Greg Brown <[email protected]>
Most of the time my applications don't have any shutdown logic.
So the
shutdown() method is already a corner case.
Actually, most of our sample applications do. They close the
main
window,
perform service logout, etc.
there are going to be times when the program does not have a
choice about shutting down (e.g. System Update forced a reboot).
Thus we need to separate the "can shutdown" flag from the
"shutting
down
now" action.
That's why shutdown() takes a boolean "optional" argument. If
optional is
true, the method can return a value indicating whether it would
like to
cancel shutdown. If false (e.g. during reboot), the return
value will
be
ignored.
I think the method signatures as currently defined are OK. They
parallel
(somewhat) the applet lifecycle methods. My question to you all
was -
should
the return value of shutdown() mean "approve" or "deny"? It is
currently
defined as "approve", but I'm leaning towards changing it to the
latter.
