Does checking for the Core.Instance.isLocalPlayer twice in my method constitute a bad design?

Question

I've been writing some code for my unity project, where I am connecting players to a game and adding them to a dictionary, which is in a Database class.

I have a simple method which is adding them to the database and if successful, it then instantiates the relevant game objects into the game.

The thing that made me wonder if it was a bad design was the way I was checking if the player is the local client or not.

This is my code:

public bool SetConnection(byte connectionID)
{
    Agent agent;
    if (Core.Instance.isLocalPlayer)
        agent = new Agent_Local();
    else
        agent = new Agent_Enemy();

    if (Core.Instance.Database.Add(connectionID, agent)) // fails if room is full
    {
        if (Core.Instance.isLocalPlayer) //Checking this again : maybe bad design?
            Core.Instance.LocalHandler.SetPlayer(agent);
        else
            Core.Instance.SetEnemy(agent);

        return true;
    }
    return false;
}

So you will notice I am checking if the player is local twice, and I feel that I shouldn't need to check if I was smarter about the way I went about setting this up. Is this considered as bad or redundant coding or perhaps someone could suggest a better approach?

Answer 1

If you find yourself using a lot of if (type == x) do x else do y statements you can use inheritance. The problem with using if (type == x) throughout your code is that if tomorrow you add a new Agent, you have to add an extra else case in each place you do this.

Here's an example of how to improve that:

public bool SetConnection(byte connectionID)
{
    Agent agent = agentFactory.CreateAgent(Core.Instance.IsLocalPlayer);
    return agent.TryToEnterRoom(connectionId);
}

Answer 2

In your case it's OK. But I would store Core.Instance to local variable since it is used multiple times:

public bool SetConnection(byte connectionID)
{
    var core = Core.Instance;

    Agent agent = core.isLocalPlayer
        ? new Agent_Local()
        : new Agent_Enemy();

    if (!core.Database.Add(connectionID, agent))
        return false; // yes, I'm fan of early returns :)

    if (core.isLocalPlayer)
        core.LocalHandler.SetPlayer(agent);
    else
        core.SetEnemy(agent);

    return true;
}