-3

I have this inteface:

public interface ICommand
{
    bool Execute(Vector2 commandPosition);
    bool Execute(GameUnit gameUnit);
    bool Execute(string shortcut);
}

And a class with these methods making same things with different argument types

private void DispatchGameUnitCommand(GameUnit gameUnit)
{
    if (_preparedCommand != null)
    {
        _preparedCommand.Execute(gameUnit);
        return;
    }
    for (int i = 0; i < _commands.Length; i++)
    {
        if (_commands[i].Execute(gameUnit))
        {
            return;
        }
    }
}

private void DispatchLocationCommand(Vector2 screenPosition)
{
    if (_preparedCommand != null)
    {
        _preparedCommand.Execute(screenPosition);
        return;
    }
    for (int i = 0; i < _commands.Length; i++)
    {
        if (_commands[i].Execute(screenPosition))
        {
            return;
        }
    }
}

private void DispatchShortcutCommand(string shortcut)
{
    if (_preparedCommand != null)
    {
        _preparedCommand.Execute(shortcut);
        return;
    }
    for (int i = 0; i < _commands.Length; i++)
    {
        if (_commands[i].Execute(shortcut))
        {
            return;
        }
    }
}

How could I improve them removing duplicated code? Is it possible in anyways?

ProtoTyPus
  • 1,272
  • 3
  • 19
  • 40

1 Answers1

0

Your post says you have "a class with these methods" for GameUnit, Vector2 and string.

All indications in your code sample are that an actual GameUnit is required to do anything. In this case, the thing that "might" really help is having implicit conversion operators that turn a string or a Vector2 into a GameUnit.


GameUnit with implicit conversions

public class GameUnit
{
    public Vector2 Vector2 { get; set; }
    public string? Name { get; set; }
    public static Dictionary<string, GameUnit> Shortcuts { get; }
        = new Dictionary<string, GameUnit>();

    public static implicit operator GameUnit(Vector2 vector)
    {
        return new GameUnit { Vector2 = vector };
    }

    public static implicit operator GameUnit(string shortcut)
    {
        Shortcuts.TryGetValue(shortcut, out GameUnit gameUnit);
        return gameUnit; // Will be null if not in dict
    }

    public override string ToString() => $"{Vector2} {Name}";
}

Following this logic, we end up with a dispatcher class that needs only one method not three.

public class Dispatcher
{
    public void DispatchGameUnitCommand(GameUnit context)
    {
        Console.WriteLine($"Dispatching {context}");
    }
}

As for the interface, how were you planning on using it? So far, it's not needed for anything.


Testing

static void Main(string[] args)
{
    Console.Title = "GameUnit Test";
    Dispatcher exec = new Dispatcher();
    GameUnit
        adHoc = new GameUnit { Name = "AdHoc", Vector2 = new Vector2(5000, 2000) },
        vectorA = new GameUnit { Name = "VectorA", Vector2 = new Vector2(5, 20) };
    GameUnit.Shortcuts[adHoc.Name] = adHoc;
    GameUnit.Shortcuts[vectorA.Name] = vectorA;

    exec.DispatchGameUnitCommand(adHoc);
    exec.DispatchGameUnitCommand("VectorA");
    exec.DispatchGameUnitCommand(new Vector2(500, 200));
}

console output

IVSoftware
  • 5,732
  • 2
  • 12
  • 23
  • As an aside, consider renaming `ICommand` to something (anything) else. It's likely to cause massive conflicts with `System.Windows.Input.ICommand` going forward. – IVSoftware Feb 04 '23 at 11:59