[
https://issues.apache.org/jira/browse/HADOOP-7202?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13010771#comment-13010771
]
Tanping Wang commented on HADOOP-7202:
--------------------------------------
I looked at the patch and think it looks good to me with only a few comments,
1) In PathData.java, class member exists which is a Boolean variable, its
value is determined by checking if FileStatus is null. Since FileStatus is
another class member, by checking whether FileStatus is null, we can already
know if the Path exists. So I think we can eliminate this "exist" member
variable?
2) In Command.java#displayError(Exception e)
{code}
if (errorMessage == null) {
errorMessage = e.toString();
}
{code}
StringUtils.stringifyException() is a common utility function which makes a
string representation of the exception. I think using this is better than just
toString()
Also, it would be a good idea to log a debug message using LOG.debug to print
out the full exception stack trace, so that debugging is easier.
3) In Command.java#run()
{code}
* @throws IllegalArgumentException if the option parsing fails
{code}
would it be a good idea to link the function, so it is more clear? i.e.
* @throws IllegalArgumentException if {@link processOptions(args)} fails
I am confused about the other throws comment
{code}
* @throws non-IOExceptions such rpc failures
{code}
4) In Command.java#run(String, cmd, LinkedList<String> args)
the exit code of error was -1 in the original code, it is now changed to 1.
Although it does not matter as long as it is non-zero value, why not keep it
the same?
5) You are making the methods in PathData.java public, just making it package
default by removing the "public" should be sufficient enough in our case?
> Improve Command base class
> --------------------------
>
> Key: HADOOP-7202
> URL: https://issues.apache.org/jira/browse/HADOOP-7202
> Project: Hadoop Common
> Issue Type: Improvement
> Affects Versions: 0.22.0
> Reporter: Daryn Sharp
> Assignee: Daryn Sharp
> Fix For: 0.23.0
>
> Attachments: HADOOP-7202.patch
>
>
> Need to extend the Command base class to allow all command to easily subclass
> from a code set of code that correctly handles globs and exit codes.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
