[Engine-devel] a different approach to the command classes

[Jon Choate]

The way the command classes are written has bothered me for a while.  
While implementing the multiple storage domain features I am presented 
with the opportunity to create a new command from scratch.  I gave some 
thought to what I would like the command classes to look like while 
balancing that the class must still fit in with the existing structure.  
So here is what I came up with. I'd appreciate any feedback.

The Command class encompasses only the rules of what needs to be done.  
It relies upon Validator classes to determine if the canDoAction 
conditions have been met.

     @Override
     public boolean canDoAction() {
       ...
         checkTargetDomainHasSpace();
         checkTargetDomainIsValidTarget();
         checkSourceDomainIsValidSource();
         checkSourceAndTargetAreDifferent();
      ...
}

...

   private void checkTargetDomainHasSpace() {
         if(!actionAllowed) return;
         
if(!targetDomainValidator.hasSpace(getParameters().getDiskImageId())) {
           
addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_DISK_SPACE_LOW);
           actionAllowed = false;
         }
     }


Each of the checks follows a similar pattern of
     - guard clause to see of we already failed and bail if we did
     - test for failure of the condition
     - add failure message if needed
     - set variable to failed if needed

Storing the success flag in a variable allows us to keep the canDoAction 
method readable as a series of commands and to allow it to be accessed 
by all the private methods without them having to pass it around.

The execution of the command will follow a similar pattern where the 
command class will only know how to describe what needs to be done and 
to rely on supporting objects to handle the implementation of these 
steps.  Getting the implementation out of the command classes will allow 
the commands to share validation and implementation details and remove a 
lot of the duplication that currently exists within the commands.


How do people feel about this approach?