Hi Alan,
For something to log from an inoperative destroy call, are you thinking
it should
throw an exception? In that case the return value could be void and the
exception
message could expose the OS specific reason.
None of the currently available exceptions seem appropriate;
A bare RuntimeException might do but perhaps a new exception class is
needed.
Thanks, Roger
On 5/14/15 8:30 AM, Alan Bateman wrote:
On 13/05/2015 16:10, Roger Riggs wrote:
Hi Alan,
Yes, the destroy use looks a bit odd. In Process, destroyForcibly
returns this so that the application can fluently wait for the
termination to complete.
Returning Optional<ProcessHandle> enables the case to fluently
perform an action on the process when it does exit.
ProcessHandle ph = ...;
ph.destroy().ifPresent(p -> p.onExit( ...));
Optional.isPresent() provides a boolean if that's needed.
Optional is good and the usages with parent(), etc. are nice update to
the API. Chaining methods invocations is good too.
It's just destroy that seems a bit much as it's too easy to mix up
"present" and "alive", e.g.: the process was present so we attempted
to destroy it, it may or may not be present now. In that context using
ifPresent in this example is confusing to read because it's really
"was present, on its way to not being present" (if you know what I
mean). That's the only one that might need to be looked at again to
see if there are better candidates. Related to this is that the
destroy methods don't have a way to communicate anything useful to log
when they fail. With Process no long implementing ProcessHandle then
there is an opportunity to look at that again.
-Alan.
