admin@glassfish.java.net

Re: code review for issue 694

From: Kedar Mhaswade <Kedar.Mhaswade_at_Sun.COM>
Date: Fri, 22 Sep 2006 05:39:35 -0700

> If the array of args is passed then the caller method will look like this:
>
> (1) chmod(new String[] {"-R", "u+x"}, layout.getBinDir());
> instead of this:
> (2) chmod("-R u+x", layout.getBinDir()); <--- this is how it is
> called in PEDomainsManager.java
>
>
> Doesn't (2) look more pleasant? :-)

The question/comment was not about it being pleasant to the eyes :)
The problem with the single line is that of a space or other white space
characters. It is always better to use the form that takes String[].

Going by the same token, I'd say Runtime.exec should not have provided
the overloaded method that takes a single String. In that regard too,
ProcessBuilder is a better choice.
>
>
> Also, in a separate e-mail to Byron, I told him that I don't know if I
> can log a warning message in server.log. This code is in servermgnt.
> I'll add the code to check if file exists, but I'd need to wrap the
> message in an exception so it is handled by the caller which eventually
> is CLI. CLI will display the message in stdout.

Yes, there is no way to log a message to server.log from here. This is
primarily a client side code. But there too, please don't make any
assumptions about callers. In the future, there could be a caller of this
code that is not CLI.

>
> Jane
>
>
>
>
>
> Byron Nevins wrote:
>
>> I would wager that there /are/ no subclasses in other packages. If
>> so, this is a case where adding "protected" made maintenance harder
>> and more expensive.
>>
>> Ref: http://blogs.sun.com/foo/entry/questionable_use_of_protected_in
>>
>>
>> Bill Shannon wrote:
>>
>>> Byron Nevins wrote:
>>>
>>>> * It's too bad that the chmod method is declared as protected
>>>> rather than as nothing (package-private). If it was -- you could
>>>> just change the caller code (3 places in the package) to pass in
>>>> an array of args instead of doing all of that painful
>>>> processing. But, since it is protected there could be another
>>>> caller of the
>>>> chmod anywhere in the other millions of lines of code, so forget
>>>> this idea! Moral for interested readers: don't use protected
>>>> unless it is necessary.
>>>
>>>
>>>
>>> Can't you find all the subclasses and fix them as necessary?
>>> This isn't a public API, right?
>>>
>>> Or, if you're really paranoid, add another chmod method that
>>> takes the arg array and change the callers to use it.
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: admin-unsubscribe_at_glassfish.dev.java.net
>>> For additional commands, e-mail: admin-help_at_glassfish.dev.java.net
>>>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: admin-unsubscribe_at_glassfish.dev.java.net
> For additional commands, e-mail: admin-help_at_glassfish.dev.java.net
>