1

I am developing a simulator of a bank software system, where each operation (deposit, withdraw, transfer, ...) is just a string.

I have 5 main classes :

Bank where bank accounts are stored.

CommandValidator that takes a command (string) then decides whether it is valid or not (through its validate(string) method) according to its bank's stored accounts.

public class CommandValidator {
  private Bank bank;
  public CommandValidator(Bank bank) {
    this.bank = bank;
  }
  public boolean validate(String command) {
    //...
  }
}

CommandProcessor that takes a command and executes it (through its process(string) method), it operates on its bank's stored accounts.

public class CommandProcessor {
  private Bank bank;
  public CommandProcessor(Bank bank) {
    this.bank = bank;
  }
  public boolean process(String command) {
    //...
  }
}

CommandStore that stores the entire system's commands.

public class CommandStore {
  private List<String> commands;
  public CommandStore() {
      commands = new ArrayList<String>();
  }
  public boolean store(String command) {
      commands.add(command);
  }
}

In order to conserve the SRP (single responsibility principle), a fifth class named Manager, must manage the operations between CommandValidator, CommandProcessor and CommandStore, by taking a new command (string), then managing validate, process and store methods' calls in takeCommand(string) method.

public class Manager {
   //...
   
   public Manager(Bank bank) {
      //...
   }

   public void takeCommand(String command) {
      //...
   }
}

But I want also to conserve the dependency injection, so I can not write the Manager constructor this way :

public class Manager {
   private Bank bank;
   private CommandValidator cv;
   private CommandProcessor cp;
   private CommandStore cs;
   
   public Manager(Bank bank) {
      this.bank = bank;
      cv = new CommandValidator(bank);
      cp = new CommandProcessor(bank);
      cs = new CommandStore();
   }
}

If I try to inject each object, I will obtain too much parameters in the Manager constructor :

 public class Manager {
       private Bank bank;
       private CommandValidator cv;
       private CommandProcessor cp;
       private CommandStore cs;
       
       public Manager(Bank bank, CommandValidator commandValidator, CommandProcessor 
                      commandProcessor, CommandStore commandStore)
          this.cv = commandValidator;
          this.cp = commandProcessor;
          this.cs = commandStore;
       }
    }

What can I do in order to conserve both SRP principle and DI principle?

X Y
  • 301
  • 3
  • 9

3 Answers3

5

There is nothing wrong with passing collaborator objects in the constructor. It's just the design itself is off. Before trying to make sense of all the "principles" and "patterns" out there, you have to get the basics right. That is object orientation.

You are using objects, but your code is not object-oriented. You are essentially camouflaging procedures as objects. I.e. CommandValidator is basically a validate() method you want to have. CommandProcessor -> process(), etc. The Manager is a big command and control object (we used to called that a god object).

Instead of thinking about what technical steps you would have to execute in sequence to solve your problem, in object-orientation you have to think about what things cooperate to solve a problem. This switch in thinking is not easy, but you absolutely have to do it. Most everything else is just confusing until you get this right.

I don't know your exact use-case, but here is an alternative:

public interface Command {
   public boolean validate();

   public void execute(Bank bank);

   ...
}
Robert Bräutigam
  • 11,473
  • 1
  • 17
  • 36
  • @XY: This is your answer right here. When object construction becomes awkward with no clear path forward, then the design of your classes might need some adjusting. – Greg Burghardt May 13 '21 at 13:30
  • 1
    *"I am developing a simulator of a bank software system, where each operation (deposit, withdraw, transfer, ...) is just a string."* -- This is the root of the problem. The problem domain was modeled incorrectly to begin with. In banking, everything is basically a transaction. The CommandValidator, CommandStore, CommandProcessor and Manager classes make no sense in the problem domain of banking. – Greg Burghardt May 13 '21 at 13:35
  • +1. Also, [parse, don't validate](https://lexi-lambda.github.io/blog/2019/11/05/parse-don-t-validate/). Or more generally: make illegal states unrepresentable. OP should distinguish between a command interchange format (might be some string) and the internal representation. It should not be possible to create a command object that's invalid, which can drastically simplify internal code. But this requires proper boundaries (in the DDD sense) between “inside” and “outside”. – amon May 13 '21 at 13:59
  • @Greg Burghardt I deal with you that I am trying to camouflage some procedures as objects, also that a command must not be just a string. But it is just a homework, where profs have chosen that. So I must continue according to that choice. – X Y May 13 '21 at 20:23
4

You can take a deep breath and think about what you want to achieve instead of what principles you want to follow.

And think about whether you understood SRP. It’s no the “single method principle”.

gnasher729
  • 42,090
  • 4
  • 59
  • 119
3

Inject a factory.

It's okay to instantiate the dependencies in the factory. If someone writing a unit test wants to mock the dependencies, they can mock the factory instead. Just remember the interface.

public class ManagerFactory : IManagerFactory {
{
    public Manager CreateManager(Bank bank)
    {
        return new Manager {
            cv = new CommandValidator(bank),
            cp = new CommandProcessor(bank),
            cs = new CommandStore()
        };
    }
}
John Wu
  • 26,032
  • 10
  • 63
  • 84
  • I was thinking about something near; making a new class `BankTools` that takes a bank in its constructor, then create a CommandValidator, CommandProcessor, and CommandStore according to that bank. An object of `BankTools` will be injected in the `Manager`. – X Y May 13 '21 at 20:19
  • If `BankTools` takes `bank` in its constructor, how do you plan to inject it into `Manager`? – John Wu May 13 '21 at 20:26
  • By making the `Manager` constructor take a `BankTools` object. Manager manager = new Manager( new BankTools(bank) ); Does it solve the problem? – X Y May 13 '21 at 21:27