Hi Jane,
Sorry for taking time. And I think the signature of chmod is
outside the changes that you are doing. You are just changing
the body of a method that already exists. So, I think you can
go ahead with your changes.
But believe me, time spent now will be more time saved in future.
Regards,
Kedar
Jane Young wrote:
> Hi Kedar,
>
>>
>> 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[].
>
>
>
> Keep in mind that majority of the calls to chmod method are with a
> single arg. eg. chmod("600", pwdFile); <--
> MasterPasswordFileManager.java,
> Personally, I don't like creating a String array with just one element.
> I can provide a overloading method "chmod" that takes a String array.
> But do you really think it is necessary in this case?
>
> I want to move on and check in with this bug fix.
> We are boggled with the method defined as "protected", but as it turned
> out it is called from a different package and then the argument as array.
> It's good that we have a detailed code review, but I didn't expect it to
> be this long....
>
> Can someone just tell me if my changes are okay? Should I have another
> chmod method to handle array arg?
>
> Thanks,
> Jane
>
>
>>
>> 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
>>>
>>
>> ---------------------------------------------------------------------
>> 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
>