0

Say, I have an agent whose state includes position and facing_direction. I need to check whether it can move in any direction or is completely stuck (due to various obstacles). Agents have a method can_move() that checks if the agent can move forward, i.e., along its facing_direction.

I can modify the agent's facing_direction to go through all possible directions in a loop, and then call can_move(). At the end, I would return the agent's facing_direction to its original value. However, this feels somewhat awkward, since I'm changing the state only to perform a calculation, not to actually turn or move the agent. Is this approach considered to be an acceptable technique? Does it have any disadvantages (in a single-threaded program)?

The alternative of course is to refactor can_move() method to accept the direction as a parameter. This would involve copying that parameter across several methods involved in the calculation, and risking confusion between this parameter and the actual facing direction stored on the agent instance.

I'm using python but I doubt this question would have different answers in different languages as long as it's not a purely functional language.

2
  • What are you doing with the information from can_move? Are you immediately picking a possible direction and move that way? Or are you showing the user the possibilities he has? Something else? – Bart van Ingen Schenau Jan 25 '17 at 8:29
  • @BartvanIngenSchenau After I collect all the answers from can_move() (for each direction), I return True if the agent cannot move anywhere (False otherwise). This information is then used to modify the agent's future behavior. So there's no direct connection between a call to can_move() and a movement. – max Jan 25 '17 at 8:39
3

Your current solution sounds like a hack, for two reasons:

  • You are asking the agent for some information, but to do that you also have to issue commands like “turn left”. It is generally better to practice command–query separation since that makes it clearer when a method call will change the state of the object.

    In itself, changing the agent's state to probe into different directions is not problematic since the agent will face into the same direction before and after the method call. However, changing object state intermittently introduces fragility: What happens when an exception is thrown before the agent has returned to their original orientation? Does the post-condition “the agent faces in the original direction” hold then as well? What happens if the number of turns is associated with some cost, or is tracked? E.g. if this is a puzzle game, the game score might consider the number of turns. Implementing the is-stuck check in terms of turning the agent might directly influence the game score.

    Note that in Python, setting an instance field may very well have unknown side effects if that instance field is implemented as a property. In contrast, setting a member field is always “safe” in a language like Java.

  • The operations provided by the agent and the operations required in your program are mismatched. You want to know whether an agent is stuck, so it should have a method agent.is_stuck() -> bool. Since that method is internal to the agent, it can directly use the same logic as agent.can_move_in_facing_direction() with no need to modify the object state. You would probably extract the common logic into a helper method like self.__can_move_in_direction(d).

As a general design issue, using anything except function parameters to pass function parameters is not a good idea. Using instance fields is not fundamentally different from using global variables to communicate parameters. It's the moral equivalent of:

# params for add
x = None
y = None
result = None

def add():
  result = x + y

# invoke add
x = 40
y = 2
add()
print(result)  #=> 42

Does this have its uses? Yes, in space-constrained assembly programming where you have to prevent stack allocations. But in high-level languages? Not really.

The problem with global variables (or really, any variable with a large scope) is that it's increasingly difficult to keep track of how that variable is used. If I use a global-ish variable and call a function, I need to know whether the called function directly or indirectly modifies that variable. Keeping track of the exact behaviour of all those functions ranges from difficult (if there are many functions that could affect those variables) to impossible (if infinitely many functions could be added by inheriting your class). It's difficult to understand the behaviour of these functions since they don't just depend on their explicit parameters, but implicitly also on the state of the world around them.

Mutating state as little as possible and keeping any mutable state nicely encapsulated and divided into small cohesive chunks is not just a matter of “clean code”, it's fundamental to keep a large code base comprehensible.

1
  • The passing of parameter d would be required to several layers of functions (since can_move_facing_direction calls other methods, which originally relied on reading direction information from the instance). But i agree, it's better than the messy alternative I suggested. – max Jan 25 '17 at 16:08

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.