Coding style - Check conditional inside function or before running it?

Should the IsAttacking() be checked inside the function, or before running the function? How do you normally do it?

 if (Input.GetButtonDown("Action"))
	{
		if (!attackScript.IsAttacking())
			attackScript.DoAttack();
		else
			Debug.Log("Player tried to attack, but is already attacking.");
	}

	public void DoAttack()
	{
		// ATTACK CODE
	}

Or…

	if (Input.GetButtonDown("Action"))
	{
		attackScript.DoAttack();
	}

	public void DoAttack()
	{
		if (!IsAttacking())
			// ATTACK CODE
		else
			Debug.Log("Can't attack, already attacking.");
	}

Is the calling code concerned with whether it is attacking? If not the check should be inside. If the outside code should not be concerned with the implementation of the method you are calling it should be inside.

A more thorough explanation is on programmers stack exchange.

One rule of thumb for reusable, library-style classes is that private methods can make assumptions about internal state and/or the state of arguments, because their callers know everything, but public methods cannot because their callers often can’t even see the internal state. Encapsulate the constraint in the class that best understands it.

Extending that, it can be important for public methods to perform these checks in order to guarantee that the internal state of the object remains consistent. It is less important (but still often valuable) for private methods to perform these kinds of checks, because they can trust the caller to understand the internal state itself.

However, for the case you give, I think your class really ought to let the calling code see that attacking right now is futile. Then the calling code has an opportunity to choose to do something more useful instead, rather than just issuing an attack order that is ultimately ignored. For this kind of thing, think of the principles behind the HATEOAS model - code shouldn’t assume it knows what can be done next, it should ask the thing it’s going to talk to what options are available, then pick one. This needs to be applied selectively, but I think game “AI” is one area that this logic applies well.

I’d say first method is better as it doesn’t go to the function unless the condition is met.
You save time and time is precious :slight_smile: