Justification for doX Processing Target Strings

One of the rules of coding that I try to abide by, both at work and at home, is always to have a reason for doing something some way. Whether it be choosing to use if/elsif vs. switch statements or whether to make something a member of some class, I try to have a reason for every choice that I encounter. Sometimes I get distracted or something, and make what I later feel to be a bad choice. When correcting it, again, I require myself to have a justification. Here is one such time.

In the sequence of events involved with commands it actually seems to handwave about the final step. Ultimately a method is invoked on the character object that is performing a command that actually has the implementation of the action that takes place. For instance, if you type, "say hi", at some point character.doSay("hi") is called. There's one such so-called doX method for each applicable command. Certain commands take targets. For instance, when you type, "get bag", something needs to interpret "bag" as an identifier in the game world and associate it with a real Dealie instance, which is then manipulated by doGet. In this example, "bag" is what I refer to as a "target string."

Originally, the GetCommand class would do a minimal amount of processing on the arguments to the command, then pass on basically the raw target string to doGet, which would lookup the target string in the current room, find the dealie, and mess with it.

At some point, I got the crazy idea that the doX methods Really Should take the final objects themselves. For example, whereas the signature for doGet was something like doGet(targetString) before, it then became doGet(targetDealie). The reason I came up with was that...er..."it just seems right," or some nonsense like that. OK, so I don't think I had a really solid reason for it.

So I did a bunch of work to push out most of the target string to target object code into the Command classes. In a way, this wasn't so bad, because this code was kind of old and it got a well-deserved polishing. Then I came to my senses when I realized a few reasons why it shouldn't be that way.

Firstly, it violated one of Scott Meyers' rules about good design: make interfaces easy to use correctly and hard to use incorrectly. Before, a caller passed in simply a string, and it was thoroughly validated within the doX method. Afterwards, the caller could pass in anything they really wanted, and its validity needed to be thoroughly validated within. A lot of the checks that are done in the process of resolving the target string are kind of implicit, and making them explicit was not only tedious, but also prone to becoming outdated with the arrival of new features, and probably duplicate code for something the caller does to perform the mapping himself.

For example, in the "get" example above, the target string is looked for only on the ground of the room the character is standing in. After my silly change, the caller can pass in an object that is already in the character's inventory, which is a clearly invalid case. The root cause of this class of problems is that the real logic of the functionality is now spread between two different functions--one that does the target string lookup, and one that does the processing on the target; but these pieces of logic are closely tied together, and so should really live close together.

Secondly, the previous way represented the behavior of a character better. A command like, "get 3.bag" means something like, "get the third thing identified as a bag nearby [in this room]." Then code within the Mob class is responsible for finding the real object--i.e. looking for it with his eyes--and picking it up. The newer way detached these steps from the Mob class and put them in the Command classes instead, which have no similar analog in the MUD world.

Finally, the newer way violates the Law of Demeter worse than the older way did. I have a lot of places in my code where I disobey the Law of Demeter, but reducing those occurrences is widely considered a good thing to do. Basically, the Law of Demeter says that objects shouldn't really try to access objects that aren't direct "neighbors" in some sense. Specifically, the code in the Command classes would do things like query the character for what room they're in and call functions on that to look up dealies or mobs in that room. This kind of interaction between the Command classes and things that really don't concern them (rooms, dealies, etc.) leads to tight coupling, which eventually becomes hard to break.

But I wasn't able to remove this entirely. There is a reason to support these kinds of calls receiving target objects directly. Here's an example. A certain nipic is programmed to whisper some words to anyone who whispers something to them. More specifically, when this nipic receives any WhisperEvento, he needs to whisper some string to evento.source, which is generally an instance of Mob. With the old way, the rough code to do this would look like so:

def receiveEvento(evento)
    if evento.source != manager().mob
        if evento.kind_of?(WhisperEvento)
            manager().mob().doWhisper(mapTargetToTargetString(evento.source), "Don't whisper at me!")
        end 
    end 
end # function receive evento

As you can see here, in order to whisper to someone whom the nipic already has unambiugously identified, the code is performing a reverse mapping, from unambiguous source object to more ambiguous target string. If you imagine that the source's short description is "a towering man wearing glasses," that target string might look like "4."man big giant glasses"", which includes all of its keywords in an attempt to make it as precise as possible.

Compare that with the actual final solution that I implemented:

:
manager().mob().doWhisper_Direct(evento.source, "Don't whisper at me!")
:

The only problem is the inability to restrict who can call the doX_Direct methods. They're not just a performance optimization, but also help with correctness in certain situations, where validity checking has already taken place.