admin@glassfish.java.net

Re: Code Review Needed

From: Byron Nevins <Byron.Nevins_at_Sun.COM>
Date: Mon, 30 Oct 2006 19:18:57 -0800

Bill Shannon wrote:
> Byron Nevins wrote:
>> This is a new utility class. It is designed for taking the drudgery
>> out of attaching threads to stdout and stderr of processes created in
>> java.
>
> Sure would be nice if it followed our coding conventions...
Is there something besides the placement of opening curly braces? I
can't code with them placed raggedly as per the convention. But I *can*
check it in that way when I'm done.
>
> There seems to be no synchronization of the objects shared between the
> worker threads and the calling thread.
There are no shared objects, or at least there is no shared /changing
/of objects. I don't see any potential problems. Do you see a problem???
>
> None of the output streams opened are ever closed.
good point. It is impossible/messy to do it perfectly, though, because
once the caller starts draining he may never call back in (e.g.
waitFor()).
A better solution is to not create Streams. I've fixed this problem by
removing the methods that take File objects. The caller will have to
create an OutputStream for a File and then pass that in instead. The
caller is then responsible for closing the stream.

>
> If I ask for stdout and stderr, I think I want them interleaved in time,
> not accumulated separately and concatenated.
It's too difficult/messy to do that. The caller can instead create the
process with ProcessBuilder, use ProcessBuilder.redirectErrorStream()
and then grab the stdout String (getOut()) for interleaved output.

>
> The example in the javadocs uses different method names.
Oops. Fixed.
>
> Rather than creating threads directly, should it be using an Executor?
Maybe if I had remembered that such a thing exists :-) . I'll look into
it...
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: admin-unsubscribe_at_glassfish.dev.java.net
> For additional commands, e-mail: admin-help_at_glassfish.dev.java.net
>