A bunch of code cleanup #40
Conversation
…ups of dates, floats, and ints to be ignored
| @@ -575,8 +579,7 @@ public void setRequiredVersion(double requiredVersion) | |||
| * to copy their own data members | |||
| * @return A copy of this object | |||
| */ | |||
| // TODO: For next major release, genericize this return value (Command<ResponseType>) | |||
| public Command copy() | |||
| public Command<?> copy() | |||
There was a problem hiding this comment.
ResponseType doesn't work here?
There was a problem hiding this comment.
I was actually going to propose that remove the 'copy' method altogether. It is very inconsistently implemented and doesn't seem to provide much utility other than to pipe Command.getRequiredVersion through to response classes that care about it.
If anything actually wanted to extract the source command from an arbitrary response object, there's no guarantee that it would actually match the original command (by type or functionality) or that it is a copy.
I was also toying with the idea of making Command and PostCommand abstract (along with createResponse) then move the default createResponse to subclasses for use by things that don't want to worry about response objects.
e.g.
class BasicCommand extends Command<CommandResponse>
-and-
class BasicPostCommand extends PostCommand<CommandResponse>
(Alternatively, remove the generics from Command and PostCommand and make abstract base command classes)
There was a problem hiding this comment.
I haven't looked at usages, but I'd be fine getting rid of copy() entirely. Now's a good time. I trust your judgement...
labkey-client-api/src/org/labkey/remoteapi/di/ResetTransformStateResponse.java
Outdated
Show resolved
Hide resolved
labkey-client-api/src/org/labkey/remoteapi/security/CreateGroupCommand.java
Outdated
Show resolved
Hide resolved
labkey-client-api/src/org/labkey/remoteapi/security/CreateUserCommand.java
Outdated
Show resolved
Hide resolved
labkey-tchad
force-pushed
the
codeCleanup
branch
from
e3238b1 to
0540b89
Compare
|
Is this "real good scrubbing" going to include adding the few missing javadocs so we'll stop seeing those warnings (and have all the javadoc anyone would ever want)? |
Sure! I would love to. |
|
New feature branch: #41 |
Rationale
Thought 4.0.0 might be a good time to just scrub the code real good.
Related Pull Requests
Changes
mainmethod from several commands