HandleSelectedAction Improvement?

Hoping for some feedback on my approach to the “Click UI to Select Action” lesson. My goal is to reach Hugo’s level of Clean Code silky smoothness. :wink:

I found the approach to the unit selection and selected action execution lacked a bit of clarity. I ended up going with HandleMouseInput(), which checks for mouse button down, then executes the same code as before for TryHandleUnitSelection(). It felt “smelly” to have two different methods checking for mouse button down. So my first questions is… Is that an incorrect feeling? Multiple approaches can work, so it’s not a matter of works vs doesn’t work. If we care about writing clean code, then these differences can matter and we need to train our code sniffers to detect them. Is my sniffer broken?

To me, it felt cleaner to have a method that checks for mouse input, then acts on that input. In my mind, that meant that later we could add HandleKeyboardInput() and very easily follow our input logic.

I think my version is an improvement, but would appreciate peer review and criticism so I can know if my skills are increasing. Is this code cleaner, equivalent, or was the original code cleaner?

   private void Update() {

       if (isBusy) { return; }

       HandleMouseInput();

   }

   private void HandleMouseInput() {
       if (Input.GetMouseButtonDown(0)) {

           if (TryHandleUnitSelection()) return;

           switch (selectedAction) {
               case MoveAction moveAction:
                   HandleMoveAction(moveAction);
                   break;
               case SpinAction spinAction:
                   HandleSpinAction(spinAction);
                   break;
           }
       }
   }

   private void HandleMoveAction(MoveAction moveAction) {
       GridPosition mouseGridPosition = LevelGrid.Instance.GetGridPosition(MouseWorld.GetPosition());

       if (moveAction.IsValidActionGridPosition(mouseGridPosition)) {
           SetBusy();
           moveAction.Move(mouseGridPosition, ClearBusy);
       }
   }

    private void HandleSpinAction(SpinAction spinAction) {
        SetBusy();
        spinAction.Spin(ClearBusy);
    }

Privacy & Terms