The Repo on GitHub

We should try to get our new WorldInput stuff into play pretty soon. But let’s look around and see what comes to mind.

OK, with thoughts about sloppy code in mind, let’s look at what we’ve been doing lately and see how to get it into play.

I’m pleased with the construction of the WorldInput object nest, even though I can only use it once in the real app. I do get to use it in tests, and it might be that other tests would be improved with its use. Here’s one example of using it:

    def test_new_syntax(self):
        world = World(10, 10)
        block_id = world.add_block(6, 5)
        batch_in = WorldInput() \
            .request(world.add_bot(5, 5, direction=Direction.EAST)) \
                .action('take', None) \
                .action('turn', 'SOUTH') \
                .action('step', None) \
                .action('step', None) \
                .action('drop', block_id) \
                .finish() \
            .request(world.add_bot(7, 7)) \
                .action('step') \
                .finish()
        batch_out = world.process(batch_in)
        ...

Now, of course one could have done that all in one line, but the point is that with a bit of judicious back-slashing we can get a nice-looking outline format. I do have at least one objection to the way this works:

EntityAction = namedtuple("EntityAction", "action parameter")

class EntityRequest:
    def __init__(self, entity_identifier, world_input=None):
        self.world_input = world_input
        self.entity_identifier = entity_identifier
        self.actions = []

    def add_action(self, action: EntityAction):  # <===
        self.actions.append(action)

    def action(self, action_word, parameter=None):
        self.add_action(EntityAction(action_word, parameter))
        return self

    def finish(self):
        return self.world_input

    @property
    def identifier(self):
        return self.entity_identifier

    def __iter__(self):
        return iter(self.actions)


class WorldInput:
    def __init__(self):
        self.requests = []

    def __iter__(self):
        return iter(self.requests)

    def add_request(self, request: EntityRequest):  # <===
        self.requests.append(request)

    def request(self, identifier):
        rq = EntityRequest(identifier, self)
        self.add_request(rq)
        return rq

The arrows point to the issue that troubles me: the classes know each other by name. Now they are meant to be used in this fashion and no other, so I don’t feel awful about it, but I don’t love it.

I wonder if we could build a factory object to do this same job. Shall we try it? Let’s do. I’ll start a new test file for it.

class TestWorldInputFactory:
    def test_building(self):
        world = World(5,5)
        bot_id = world.add_bot(5, 5)
        block_id = world.add_block(7,8)
        world_input = InputBuilder(world) \
            .request(bot_id) \
                .action('take') \
                .action('turn','SOUTH') \
                .action('step') \
                .action('step') \
                .action('drop', block_id) \
            .request(world.add_bot(7, 7)) \
                .action('step') \
                .action('step') \
            .result()
        assert len(world_input.requests) == 2

Here, I’ve not provided the finish method, which we didn’t ever really want, and which is probably unnecessary even in the WorldInput builder Scheme. And I’ve added the result method, so that we can return the WorldInput we built.

class WorldInputFactory:
    def __init__(self, world):
        self._world = world
        self._world_input = WorldInput()
        self._current_request = None

    def request(self, identifier):
        self._current_request = EntityRequest(identifier, self._world)
        return self

    def action(self, action_word, parameter=None):
        operation = EntityAction(action_word, parameter)
        self._current_request.add_action(operation)
        self._world_input.add_request(self._current_request)
        return self

    def result(self):
        return self._world_input

I think I prefer that to the more clever scheme that makes the classes know each other too intimately.

I’m not in love with the name WorldInputFactory. Would Builder be better?

Does it need to be named World? No, that’s a hanger on from trying to keep the two sides separate. If and when we separate the programs, calling things World this and that will be unnecessary. Rename this one and use it in the class tests as well as here.

class InputBuilder:
    def __init__(self, world):
        from test_world_batch import WorldInput
        self._world = world
        self._world_input = WorldInput()
        self._current_request = None

    def request(self, identifier):
        from test_world_batch import EntityRequest
        self._current_request = EntityRequest(identifier, self._world)
        self._world_input.add_request(self._current_request)
        return self

    def action(self, action_word, parameter=None):
        from test_world_batch import EntityAction
        operation = EntityAction(action_word, parameter)
        self._current_request.add_action(operation)
        return self

    def result(self):
        return self._world_input

Yes, I think that’s much nicer. Now we can remove the magic methods from the other classes. The result:

EntityAction = namedtuple("EntityAction", "action parameter")

class EntityRequest:
    def __init__(self, entity_identifier, world_input=None):
        self.world_input = world_input
        self.entity_identifier = entity_identifier
        self.actions = []

    def add_action(self, action: EntityAction):
        self.actions.append(action)

    @property
    def identifier(self):
        return self.entity_identifier

    def __iter__(self):
        return iter(self.actions)

class WorldInput:
    def __init__(self):
        self.requests = []

    def __iter__(self):
        return iter(self.requests)

    def add_request(self, request: EntityRequest):
        self.requests.append(request)

We should commit: convert to InputBuilder, away from clever classes.

I think I’d like to make the members private on these classes. And in general, if I were wiser than I am. (cf. sloppy coding.)

EntityAction = namedtuple("EntityAction", "action parameter")

class EntityRequest:
    def __init__(self, entity_identifier, world_input=None):
        self._world_input = world_input
        self._entity_identifier = entity_identifier
        self._actions = []

    def add_action(self, action: EntityAction):
        self._actions.append(action)

    @property
    def identifier(self):
        return self._entity_identifier

    def __iter__(self):
        return iter(self._actions)


class WorldInput:
    def __init__(self):
        self._requests = []

    def __iter__(self):
        return iter(self._requests)

    def add_request(self, request: EntityRequest):
        self._requests.append(request)

Commit: making private members. (No, you have.)

Let’s sum up.

Summary

A minor irritation, the fact that the input classes knew each other’s names, plus a bit of concern over cleverness, led us to create a factory object that centralizes any cleverness and that knows all the necessary names, while the operational classes do not know each other’s names. (Although, if we were to use type hinting … they sort of would. But they wouldn’t be using those names.)

Considering the profligate use of “World” in the names, plus the horrible tedium of typing “WorldInputFactory” more than zero times, led us to observe that putting World in the names of things is really a sop against the fact that we have both World and client bound into the same program, and we’re trying to keep the objects from crossing the client-server boundary.

I think next time we’ll change a few more names, and move some files around. Probably put world and client into separate sub folders. For now, the work of the morning is sufficient thereto. And pace Hill, things are a bit less sloppy. Just a bit tho, plenty to object to, no worries that we’ll run out.

See you next time!