admin@glassfish.java.net

Re: Code Review Needed

From: Byron Nevins <Byron.Nevins_at_Sun.COM>
Date: Tue, 31 Oct 2006 09:01:16 -0800

Bless you!


Lloyd L Chambers wrote:
> Thanks to Byron for taking the time to code something useful.
>
> Given the large amount of code with ragged right braces (more often
> than not badly misaligned), I'm all for symmetric, consistent braces,
> which leverage the innate abilities of the human brain to see
> symmetry, as compared with the semi-random ragged-right disorganized
> mess that is the alternative. Why is symmetry good in architecture,
> art, photography, and faces, but somehow inappropriate with code?
>
> IMO, comments on brace style are a waste of everyone's time, and
> *until such time as code reviews push back on the mess we already
> have, Byron's clean, readable style is a breath of fresh air*.
>
> Lloyd
>
> (recently dealing with multi-generational nastily-formatted admin code)
>
> On Oct 30, 2006, at 7:18 PM, Byron Nevins wrote:
>
>>
>>
>> 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
>>>
>