[
https://issues.apache.org/jira/browse/QPID-1522?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12662321#action_12662321
]
Rob Godfrey commented on QPID-1522:
-----------------------------------
Comments on checking r732310:
JMXConnectionFactory:
The code which looks at the exception text seems remarkable fragile to me...
At the very least I would expect some constants which could be shared with the
code that throws the exception.
Command.java:
public static String COMMAND_NAME = null;
????? Why do we need a non-final global variable in this interface, which is
initially set to null. At the very least it should have some doc in the
class... It seems like this is a bug waiting to happen. I think it is there
due to a misunderstanding of how statics work in interfaces. They are not
inherited as a template by the implementing classes. References to statics are
*always* resolved at compile time, not at run time. echo("Try `" +
COMMAND_NAME + " --help ` for more information"); in ComandImpl will *always*
evaluate to + null +
CommandExecutionEngine:
Suggestion: instead of using reflection (.newInstance) use a factory interface
and have a factory for each command type. That way you won't have to deal with
unexpected exceptions, etc...
Commandget:
Usual double license (one backdated to 2006) problem
typo: quering instead of query / return rather than returned
typo: you might be quering wrong
typo: Type Objects might not in the broker currently
Similar typos in Commandset (perhaps the typos can be factored out into some
constants/properties since the same string seem to be reproduced in many
places... and if we ever did want to provide other languages... etc)
> Refactor Command classes so that more common code is shared.
> -------------------------------------------------------------
>
> Key: QPID-1522
> URL: https://issues.apache.org/jira/browse/QPID-1522
> Project: Qpid
> Issue Type: Improvement
> Components: Java Management : CLI Tool
> Affects Versions: M4
> Reporter: Aidan Skinner
> Assignee: Aidan Skinner
> Attachments:
> 0001-QPID-1522-Raname-Command-class-to-CommandImpl-and-m.patch,
> 0002-QPID-1522-Move-common-code-up-to-CommandImpl.-Renam.patch,
> 0003-QPID-1522-Fix-spelling-error-in-classname.patch,
> 0004-QPID-1522-Move-command-line-constants-to-individual.patch
>
>
> The Command subclasses have some duplicate code which could be shared.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
