Using the Command Pattern with Parameters

Question

I have a ReloadableWeapon class like this:

public class ReloadableWeapon {

    private int numberofbullets;
    
    public ReloadableWeapon(int numberofbullets){
        this.numberofbullets = numberofbullets;
    }
    
    public void attack(){
        numberofbullets--;
    }
    
    public void reload(int reloadBullets){
        this.numberofbullets += reloadBullets;
    }
}

with the following interface:

public interface Command {
    void execute();
}

and use it like so:

public class ReloadWeaponCommand implements Command {

    private int reloadBullets;
    private ReloadableWeapon weapon;
    
    // Is is okay to specify the number of bullets?
    public ReloadWeaponCommand(ReloadableWeapon weapon, int bullets){
        this.weapon = weapon;
        this.reloadBullets = bullets;
    }
    
    @Override
    public void execute() {
        weapon.reload(reloadBullets);
    }
}

Client:

ReloadableWeapon chargeGun = new ReloadableWeapon(10);
Command reload = new ReloadWeaponCommand(chargeGun,10);
ReloadWeaponController controlReload = new  ReloadWeaponController(reload);
controlReload.executeCommand();

I was wondering, with the command pattern, with the examples I've seen, other than the object that the command is acting on, there are no other parameters.

This example, alters the execute method to allow for a parameter.

Another example, more close to what I have here, with parameters in the constructor.

Is it bad practice/code smell to include parameters in the command pattern, in this case the constructor with the number of bullets?

Answer 1

I don't think adding parameters into execute will be bad design or violate command pattern.

It totally depends on how you want to use Command Object: Singleton Or Prototype scope.

If you use Prototype scope, you can pass command parameters in Constructor methods. Each command instance has its own parameters.

If you use Singleton scope (shared/reused instance), you can pass command parameters in execute method. The singleton of the command should be thread safe for this case. This solution is a friend of IoC/DI framework too.

Answer 2

The very purpose of this pattern is to allow to define actions, and to execute them later, once or several times.

The code you provide is a good example of this pattern : you define the action "reload", that charges the gun with an amount of bullets=10 ammunition.

Now, if you decide to modify this code to add bullets as a parameter, then you completely lose the purpose of this pattern, because you will have to define the amount of ammunition every time.

IMHO, you can keep your code as it is. You will have to define several ReloadWeaponCommand instances, with different value of bullets. Then you may have to use another pattern (such as Strategy) to switch between the commands.

Answer 3

Consider a case you have 95 bullets in hand in starting, and you have made 9 commands with 10 bullets and 1 command with 5 bullets. And you have submitted these commands to Invoker, now invoker doesn't have to worry about how much bullets are left. He will just execute the command. On the other hand if invoker has to provide the no of bullets at run time then it could be the case supplied number of bullets are not available.

My point here is that Invoker must not have to worry about any extra information needs to execute the command. And as mentioned in wiki "an object is used to encapsulate all information needed to perform an action or trigger an event at a later time"

Answer 4

Using the Command Pattern with Parameters

Consider the related 'Extension Patterns' in order to hold to a Top-Down Control paradigm 'Inversion of Control'.
This pattern, the Command Pattern, is commonly used in concert with the Composite, Iterator, and Visitor Design Patterns.
Commands are 'First Class Objects'. So it is critical that the integrity of their encapsulation is protected. Also, inverting Control From Top Down to Bottom Up, Violates a Cardinal principle of Object Oriented Design, though I see people suggesting it all of the time...

The Composite pattern will allow you to store Commands, in iterative data structures.

Before going any further, and while your code is still manageable, look at these Patterns.

There are some reasonable points made here in this thread. @Loc has it closest IMO, However, If you consider the patterns mentioned above, then, regardless of the scope of your project (it appears that you intend to make a game, no small task) you will be able to remain in control of lower-level dependency. As @Loc pointed out, with 'Dependency Injection' lower class Objects should be kept 'in the dark' when it comes to any specific implementation, in terms of the data that is consumed by them; this is (should be) reserved for the top level hierarchy. 'Programming to Interfaces, not Implementation'.

It seems that you have a notion of this. Let me just point out where I see a likely mistake at this point. Actually a couple, already, you are focused on grains of sand I.e. "Bullets" you are not at the point where trivialities like that serve any purpose, except to be a cautionary sign, that you are presently about to lose control of higher level dependencies.

Whether you are able to see it yet or not, granular parts can and should be dealt with at higher levels. I will make a couple of suggestions. @Loc already mentioned the best practice 'Constructor Injection' loosely qualified, better to maybe look up this term 'Dependency Injection'.

Take the Bullets for e.g. Since they have already appeared on your scope. The Composite Pattern is designed to deal with many differing yet related First Class Objects e.g. Commands. Between the Iterator and Visitor Patterns you are able to store all of your pre-instantiated Commands, and future instantiations as well, in a dynamic data structure, like a Linked List OR a Binary Search Tree even. At this point forget about the Strategy
Pattern, A few possible scenarios is one thing, but It makes no sense to be writing adaptive interfaces at the outset.

Another thing, I see no indication that you are spawning projectiles from a class, bullets I mean. However, even if it were just a matter of keeping track of weapon configurations, and capacities(int items) (I'm only guessing that is the cause of necessary changes in projectile counts) use a stack structure or depending on what the actual scenario is; a circular queue. If you are actually spawning projectiles from a factory, or if you decide to in the future, you are ready to take advantage of Object Pooling; which, as it turns out, was motivated by this express consideration.

Not that anyone here has done this, but I find it particularly asinine for someone to suggest that it is ok to mishandle or disregard a particular motivation behind any established (especially GoF) Design pattern. If you find yourself having to modify a GoF Design pattern, then you are using the wrong one. Just sayin'

P.S. if you absolutely must, why don't you instead, use a template solution, rather than alter an intentionally specific Interface design;