- Home /
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.");
}
How many different places and scripts will DoAttack be run from? How much chance is there that someone will forget to run the check? Will you ever want to force an attack?
The most official answer I know of, like out of a textbook, is to check BOTH places. It doesn't cost much, crashes are bad, and it saves lots of "but I thought you did it?" problems.
Answer by steakpinball · Feb 23, 2015 at 09:16 PM
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.
Answer by gfoot · Feb 24, 2015 at 02:39 AM
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.
Answer by DGKN · Feb 23, 2015 at 07:48 PM
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 :)
I guess it's more flexible too, because the conditions aren't hardcoded into the function.