Robot 27
Let’s turn around and go backward, and see whether we begin to see some improvements to how we do these things. RELEARNING: I need smaller steps.
The chosen way of working here chez Ron is to build the code with reasonable craft, and to let its evolution highlight areas where the design can be improved. Opportunities are beginning to appear all around. Let’s see if we can discover some.
I think if we implement the command “back”, or whatever it’s called, and the command turn, we should drive out some interesting possibilities. We’ll start with “back”, because it should be a lot like “forward”. And we’ll start in World, and with a test.
The first thing I notice as I start to look for “forward” is that I’ve let the tidying slide. In particular, I’ve let the methods in some classes, including World, get out of alphabetical order. With a more powerful IDE, I might not care. In Codea, I do care, it’s part of finding what I’m looking for.
One moment while I move some things around … a little cutting and pasting, tests are green. Commit: Tidying alpha order.
On the subject of tidying, check out Kent Beck’s current work on tidying. Some very nice ideas in there, all pretty easy to do, to keep your workspace just a bit better.
Back Is Like Forward
It would seem that back must be a lot like forward, so let’s see how that’s done. I should mention, as I think of it, that we’re only allowing single steps, and the spec allows for any number. Maybe we’ll deal with that today, maybe not.
Here’s forward:
function World:forward(robotName,steps)
local robot = self._robots[robotName]
robot._y = robot._y + steps
local response = {
result = "OK",
data = {},
state = self:robotState(robot)
}
return response
end
I think we have a test for that. Ah, interesting. We have such a test in Robot, but not in World. Let’s do the right thing and have a test in World’s own tests.
_:test("forward adjusts position", function()
local world = World(25,25)
local rq = {
robot="xyzzy",
command="launch",
arguments={"plugh", 99,99}
}
world:request(rq)
local robotState = world:getRobot("xyzzy")
_:expect(robotState._x).is(0)
_:expect(robotState._y).is(0)
world:forward("xyzzy", 1)
_:expect(robotState._x).is(0)
_:expect(robotState._y).is(1)
end)
Close enough. It seems that almost everyone is creating a World(25,25), that should perhaps be factored out. But I’m here for bigger game.
That’s how cruft happens. We’re always after bigger game. Don’t be like my brother’s brother. Clean up things like this.
Maybe later. Trust me, I was a boy scout.
Back seems easy now:
_:test("back adjusts position", function()
local world = World(25,25)
local rq = {
robot="xyzzy",
command="launch",
arguments={"plugh", 99,99}
}
world:request(rq)
local robotState = world:getRobot("xyzzy")
_:expect(robotState._x).is(0)
_:expect(robotState._y).is(0)
world:back("xyzzy", 1)
_:expect(robotState._x).is(0)
_:expect(robotState._y).is(-1)
end)
This doubtless fails looking for back:
20: back adjusts position -- TestWorld:449: attempt to call a nil value (method 'back')
We’ll copy forward to create back.
function World:back(robotName,steps)
local robot = self._robots[robotName]
robot._y = robot._y - steps
local response = {
result = "OK",
data = {},
state = self:robotState(robot)
}
return response
end
function World:forward(robotName,steps)
local robot = self._robots[robotName]
robot._y = robot._y + steps
local response = {
result = "OK",
data = {},
state = self:robotState(robot)
}
return response
end
Test should be green. It is. Commit: World has back method.
Now we can test-drive the function in Robot.
Get Back, Robot!
We have a forward test for Robot:
_:test("Robot moves forward on 1 key", function()
local world = WorldProxy()
world:addFactAt("O",3,1)
local robot = Robot("Louie", world)
_:expect(robot:y(),"initial").is(0)
robot:scan()
_:expect(robot:factAt(3,0)).is(nil)
robot:keyboard("1")
_:expect(robot:y(), "post").is(1)
robot:scan()
_:expect(robot:factAt(3,0)).is("O")
end)
This is rather an odd test. It’s not checking the robot’s coordinates, it’s checking the view around it. This is really more of a test for Robot’s use of knowledge than of Robot’s motion.
We have another issue here, which is that we have no keystroke for going back. I propose to move toward WASD motion, as building a GUI is not on our radar. But we’ll defer this problem using robot’s existing forward and soon-to-exist back methods.
So let’s do a more direct test for forward and back.
_:test("Robot moves forward and back", function()
local world = WorldProxy()
local robot = Robot("Cyril", world)
_:expect(robot:x()).is(0)
_:expect(robot:y()).is(0)
robot:forward(1)
_:expect(robot:x()).is(0)
_:expect(robot:y()).is(1)
robot:back(1)
robot:back(1)
robot:back(1)
_:expect(robot:x()).is(0)
_:expect(robot:y()).is(-2)
end)
This of course fails for want of back in Robot. We do our copy-paste thing again.
function Robot:back(steps)
self:setResponse(self._world:back(self._name, steps))
self.knowledge = self.knowledge:newLensAt(self:x(),self:y())
end
Now the proxy will fail.
8: Robot moves forward and back -- TestRobot:146: attempt to call a nil value (method 'back')
And again with the copy-paste. This is glorious~
function WorldProxy:back(name,steps)
local result
local rq = {
robot=name,
command="back",
arguments={steps}
}
local jsonRq = json.encode(rq)
local decoded = json.decode(jsonRq)
local dName = decoded.robot
local dCommand = decoded.command
if dCommand == "back" then
local dSteps = decoded.arguments[1]
local responseDict = self.world:request(decoded)
local resp = Response(responseDict)
return resp
end
assert(false,"impossible command "..dCommand)
end
That’s exactly like the forward except for the word back. Perfect, we love duplication.
I rather expect this test to work. But I forgot a tiny detail:
Bad Result : ERROR No such command back
Ah yes. In World:
function World:requestProtected(rq)
local c = rq.command
local args = rq.arguments
local response
if c == "noResponse" then
-- nothing
elseif c == "launch" then
response = self:launchRobot(rq.robot, args[1], args[2], args[3])
elseif c == "forward" then
response = self:forward(rq.robot, args[1])
elseif c == "back" then
response = self:back(rq.robot, args[1])
else
response = self:error("No such command "..tostring(c))
end
if self:isValidResponse(response) then
return response
else
return self:error("SERVER RETURNED NO RESPONSE TO "..tostring(c))
end
end
The tests are leading us by the nose. This is why we write them. Test again, hopefully. We are green. Commit: Game understands “back” command.
There’s no way to back up … yet. Let’s modify the new test a bit:
_:test("Robot moves forward and back", function()
local world = WorldProxy()
local robot = Robot("Cyril", world)
_:expect(robot:x()).is(0)
_:expect(robot:y()).is(0)
robot:forward(1)
_:expect(robot:x()).is(0)
_:expect(robot:y()).is(1)
robot:back(1)
robot:keyboard("s") -- back: WASD
robot:back(1)
_:expect(robot:x()).is(0)
_:expect(robot:y()).is(-2)
end)
Now we’ll fail, with -1 expecting -2.
8: Robot moves forward and back --
Actual: -1,
Expected: -2
Am I hot or what??? OK, implement that in keyboard:
Well, I’m not as hot as I thought. We already use “s”, for scan. The command is look anyway, I’ll make an on-the-fly adjustment:
function Robot:keyboard(key)
if key == "l" then
self:scan() -- look
elseif key == "1" then
self:forward(1)
elseif key == "w" then
self:forward(1)
elseif key == "s" then
self:back(1)
elseif key == "r" then
local lens = RotatingLens(self.knowledge)
lens:turn("right")
self.knowledge = lens
end
end
I expect green. I am definitely not hot.
4: Robot does scan on s key --
Actual: nil,
Expected: O
Find and fix that test:
_:test("Robot does scan/look on l key", function()
local world = WorldProxy()
world:addFactAt("O", 3,0)
local robot = Robot("Louie", world)
_:expect(robot:factAt(3,0)).is(nil)
robot:keyboard("l")
_:expect(robot:factAt(3,0)).is("O")
end)
Now then: Tests are green, and I can move forward and back with the w and s keys.
Why is this OK? Well, it is mostly OK because our controls are not defined yet, we’re just using some keystrokes to demonstrate the game. So I don’t feel badly about changing the strokes, but of course at some point we need reasonable controls.
Anyway we can commit: Robot can move forward and backward with w and s keys.
Let’s calm down here and reflect.
Reflect
We’ve rather quickly implemented the “back” command, with tests in World and in Robot. And we have introduced some very significant duplication. We need to clean that up very soon.
Aside from that, things went smoothly and the tests reminded us of the various things that need to be changed to make things work. That’s what they’re good for: they show us what we need to do to implement the thing. And, if they’re small and close to the code, they tend to be robust and helpful.
Good stuff.
As for the duplication, we know that we’re going to do turn, and we can easily foresee that that’s going to change how forward and back work. Should we wait before removing the duplication?
If we were implementing move left and move right, I might be OK with getting all four moves implemented and then removing the duplication. But here, we will only ever have forward and back: turn is not a move. So let’s remove it now.
Removing Duplication
Here are forward and back in WorldProxy, where the most egregious duplication occurs:
function WorldProxy:back(name,steps)
local result
local rq = {
robot=name,
command="back",
arguments={steps}
}
local jsonRq = json.encode(rq)
local decoded = json.decode(jsonRq)
local dName = decoded.robot
local dCommand = decoded.command
if dCommand == "back" then
local dSteps = decoded.arguments[1]
local responseDict = self.world:request(decoded)
local resp = Response(responseDict)
return resp
end
assert(false,"impossible command "..dCommand)
end
function WorldProxy:forward(name,steps)
local result
local rq = {
robot=name,
command="forward",
arguments={steps}
}
local jsonRq = json.encode(rq)
local decoded = json.decode(jsonRq)
local dName = decoded.robot
local dCommand = decoded.command
if dCommand == "forward" then
local dSteps = decoded.arguments[1]
local responseDict = self.world:request(decoded)
local resp = Response(responseDict)
return resp
end
assert(false,"impossible command "..dCommand)
end
Now there’s that weird json and if going on. Those are just there because I wanted to sketch out the entire flow of commands going across the wires (or bluetooth, who knows). Let’s remove the cruft from both methods.
I do forward first:
function WorldProxy:forward(name,steps)
local rq = {
robot=name,
command="forward",
arguments={steps}
}
local responseDict = self.world:request(rq)
return Response(responseDict)
end
Wow, that’s quite a bit simpler. Test. We’re good. Could commit, but won’t. Do the same for back:
function WorldProxy:back(name,steps)
local rq = {
robot=name,
command="back",
arguments={steps}
}
local responseDict = self.world:request(rq)
return Response(responseDict)
end
function WorldProxy:forward(name,steps)
local rq = {
robot=name,
command="forward",
arguments={steps}
}
local responseDict = self.world:request(rq)
return Response(responseDict)
end
Identical except for the “back” or “forward”. Extract a method from one:
function WorldProxy:forward(name,steps)
return self:moveFB(name, "forward", steps)
end
function WorldProxy:moveFB(name,command, steps)
local rq = {
robot=name,
command=command,
arguments={steps}
}
local responseDict = self.world:request(rq)
return Response(responseDict)
end
Test. Green. Use the new method in back:
function WorldProxy:back(name,steps)
return self:moveFB(name, "back", steps)
end
function WorldProxy:forward(name,steps)
return self:moveFB(name, "forward", steps)
end
function WorldProxy:moveFB(name,command, steps)
local rq = {
robot=name,
command=command,
arguments={steps}
}
local responseDict = self.world:request(rq)
return Response(responseDict)
end
Test. Green. Commit: refactor removing duplication from forward/back.
Nice. Could be a tiny bit better, I think:
function WorldProxy:moveFB(name,command, steps)
local rq = {
robot=name,
command=command,
arguments={steps}
}
return Response(self.world:request(rq))
end
The local wasn’t explaining much of anything, so we’ll do without. Commit: inline Response creation.
I suspect we should consider having world:request return a response, but we’ll search for that duplication at another time.
Let’s see if there’s duplication in World to be improved.
Duplication in World?
function World:back(robotName,steps)
local robot = self._robots[robotName]
robot._y = robot._y - steps
local response = {
result = "OK",
data = {},
state = self:robotState(robot)
}
return response
end
function World:forward(robotName,steps)
local robot = self._robots[robotName]
robot._y = robot._y + steps
local response = {
result = "OK",
data = {},
state = self:robotState(robot)
}
return response
end
These are duplicated except for a + or -. Improvement seems possible. Extract:
function World:forward(robotName,steps)
return self:moveFB(robotName, 1, steps)
end
function World:moveFB(robotName,step, steps)
local robot = self._robots[robotName]
robot._y = robot._y + step*steps
local response = {
result = "OK",
data = {},
state = self:robotState(robot)
}
return response
end
I note that we have no tests for steps > 1. Should do that, but we can refactor, carefully, anyway. Tests should be green. They are. Use the new function in back:
function World:back(robotName,steps)
return self:moveFB(robotName, -1, steps)
end
Test. Green. Commit: Refactor forward and back for common code.
Pause, reflect.
Reflection
We pause for two reasons. One is just to settle the mind, cool down from running hot, relax after any tension that trouble may have caused. It’s a useful habit to get into: we can often start feeling a sense of urgency, and we’re likely to stumble if we let that take over.
The second reason is to look around and decide what to do next. The first idea that comes to mind while we’re coding is perhaps (!) not the best idea.
So excuse me while I have a bite of banana and a sip of chai.
Ah, that’s better.
Let’s do the “turn” function. I expect that’ll drive out some new behavior. It’ll also raise a bit of an irritation for me, which is the need to duplicate tests in World and in Robot. In principle, we could drive out World behavior using Robot’s tests … but if we were in the business of creating World, I’m not sure we’d do it that way. It’s generally better to test objects directly, or so I’ve always thought.
For now, we’ll suppose that we’ll do both layers of test, but I’m feeling duplication in my mind.
World Turn
There are a few aspects to turning. The basic one is that the robot’s “direction” will change, N to E and so on. But in addition, the scan—I really need to rename that to look—will need to return different values, and when we move, our coordinates will change differently.
I expect that the scan will be the tricky one, but we have a partially implemented solution to that, in the RotatingLens. We may use or adapt that.
Let’s do direction change, then moving, and only then, looking. Maybe we’ll rename scan this time through.
We need some tests.
_:test("turn right changes direction", function()
local world = World(25,25)
local rq = {
robot="xyzzy",
command="launch",
arguments={"plugh", 99,99}
}
world:request(rq)
local robotState = world:getRobot("xyzzy")
_:expect(robotState._direction).is("N")
end)
I expect this much to run. It does. Let’s turn. I think I’ll drive from the request this time.
_:test("turn right changes direction", function()
local world = World(25,25)
local rq = {
robot="xyzzy",
command="launch",
arguments={"plugh", 99,99}
}
world:request(rq)
local robotState = world:getRobot("xyzzy")
_:expect(robotState._direction).is("N")
local turnRq = {
robot="xyzzy",
command="turn",
arguments={"right"}
}
local resp = world:request(turnRq)
_:expect(resp.result).is("OK")
_:expect(robotState._direction).is("E")
_:expect(resp.state.direction).is("E")
end)
Kind of a big test but it tells the story. Should fail on not OK.
21: turn right changes direction --
Actual: ERROR,
Expected: OK
Other complaints arise of course but this is the big one.
Implement:
function World:requestProtected(rq)
local c = rq.command
local args = rq.arguments
local response
if c == "noResponse" then
-- nothing
elseif c == "launch" then
response = self:launchRobot(rq.robot, args[1], args[2], args[3])
elseif c == "forward" then
response = self:forward(rq.robot, args[1])
elseif c == "back" then
response = self:back(rq.robot, args[1])
elseif c == "turn" then
response = self:turn(rq.robot, args[1])
else
response = self:error("No such command "..tostring(c))
end
if self:isValidResponse(response) then
return response
else
return self:error("SERVER RETURNED NO RESPONSE TO "..tostring(c))
end
end
Testing will find that we haven’t done turn yet. Error will be the same, the protection will catch it. Implement turn:
function World:turn(robot, lOrR)
local newDir
local right = {N="E", E="S", S="W",W="N"}
if lOrR == "right" then
newDir = right[lOrR]
end
robot._direction = newDir
local response = {
result="OK",
data = {},
state=self:robotState(robot)
}
end
This is rather a lot, and it isn’t going to work, and I know at least one reason why. I expect a failure getting N expecting E.
I do get that but I get another failure first:
21: turn right changes direction --
Actual: ERROR,
Expected: OK
21: turn right changes direction --
Actual: N,
Expected: E
I think I’d like to see the error:
_:expect(resp.result).is("OK")
if resp.result ~= "OK" then
_:expect(resp.data.message).is("")
end
That’s interesting:
21: turn right changes direction --
Actual: SERVER ERROR TestWorld:275: attempt to index a string value (local 'robot'),
Expected:
Right. We’re passed the robotName, not the robot. We’re going to generate a bit more duplication here:
function World:turn(robotName, lOrR)
local robot = self:getRobot(robotName)
local newDir
local right = {N="E", E="S", S="W",W="N"}
if lOrR == "right" then
newDir = right[lOrR]
end
robot._direction = newDir
local response = {
result="OK",
data = {},
state=self:robotState(robot)
}
end
I expect the N not E error but not the other.
21: turn right changes direction --
Actual: SERVER RETURNED NO RESPONSE TO turn,
Expected:
My tests and code are smarter than I am, as it should be.
function World:turn(robotName, lOrR)
local robot = self:getRobot(robotName)
local newDir
local right = {N="E", E="S", S="W",W="N"}
if lOrR == "right" then
newDir = right[lOrR]
end
robot._direction = newDir
local response = {
result="OK",
data = {},
state=self:robotState(robot)
}
return response
end
Test. Results may vary:
21: turn right changes direction --
Actual: nil,
Expected: E
21: turn right changes direction --
Actual: N,
Expected: E
At least part of the problem is here:
function World:robotState(robot)
return {
position = {robot._x, robot._y},
direction = "N",
status = "NORMAL",
shots = robot._shots,
shields = robot._strength
}
end
We always assume N. Not a good assumption any more:
function World:robotState(robot)
return {
position = {robot._x, robot._y},
direction = robot._direction,
status = "NORMAL",
shots = robot._shots,
shields = robot._strength
}
end
I expect improvement in the tests. I do not get it unless you consider the same error twice an improvement:
21: turn right changes direction --
Actual: nil,
Expected: E
21: turn right changes direction --
Actual: nil,
Expected: E
Time to actually think, which tells me that my step was too big. We’ll reflect on that shortly. Ah.
function World:turn(robotName, lOrR)
local robot = self:getRobot(robotName)
local newDir
local right = {N="E", E="S", S="W",W="N"}
if lOrR == "right" then
newDir = right[lOrR]
end
robot._direction = newDir
local response = {
result="OK",
data = {},
state=self:robotState(robot)
}
return response
end
We don’t want to index right by lOrR, but by direction.
function World:turn(robotName, lOrR)
local robot = self:getRobot(robotName)
local newDir
local right = {N="E", E="S", S="W",W="N"}
if lOrR == "right" then
newDir = right[robot._direction]
end
robot._direction = newDir
local response = {
result="OK",
data = {},
state=self:robotState(robot)
}
return response
end
I expect green. I get it. Commit: robot direction changes on turn right.
Let’s settle down a moment.
Calm Reflection
That was too big a bite. I’m not sure what smaller bite would have been better, but I wasn’t responsive to my dear wife’s greeting, and I felt a tiny bit rattled. Not very, because I trusted my tests, but I kind of needed to debug and really should never have to do that.
Let’s improve this code a bit:
Improvement
function World:turn(robotName, lOrR)
local robot = self:getRobot(robotName)
local newDir
local right = {N="E", E="S", S="W",W="N"}
if lOrR == "right" then
newDir = right[robot._direction]
end
robot._direction = newDir
local response = {
result="OK",
data = {},
state=self:robotState(robot)
}
return response
end
What’s that bit in the middle there? It’s, I don’t know, get new direction.
function World:turn(robotName, lOrR)
local robot = self:getRobot(robotName)
robot._direction = self:getNewDirection(lOrR, robot._direction)
local response = {
result="OK",
data = {},
state=self:robotState(robot)
}
return response
end
function World:getNewDirection(lOrR, direction)
local newDir
local right = {N="E", E="S", S="W",W="N"}
if lOrR == "right" then
return right[direction]
end
end
If we had broken out that method, and tested it, we might have found that bug more quickly.
What happened was that I was writing too much code in one go, so I fumbled one of the balls, and wrote lOrR where I should have put the direction.
Let’s do a test now, and handle right as well as left.
_:test("Can get new direction", function()
_:expect(World:getNewDirection("right","N")).is("E")
end)
This test runs. Let’s complete it and then fix an obvious flaw.
_:test("Can get new direction", function()
_:expect(World:getNewDirection("right","N")).is("E")
_:expect(World:getNewDirection("right","E")).is("S")
_:expect(World:getNewDirection("right","S")).is("W")
_:expect(World:getNewDirection("right","W")).is("N")
_:expect(World:getNewDirection("left","N")).is("W")
_:expect(World:getNewDirection("left","W")).is("S")
_:expect(World:getNewDirection("left","S")).is("S")
_:expect(World:getNewDirection("left","E")).is("N")
end)
I expect the lefts to all fail. They do but they also tell me I have a duplicated (and incorrect) result.
_:expect(World:getNewDirection("left","S")).is("E")
Test again, verifying the lefts are all wrong.
Fix the method:
function World:getNewDirection(lOrR, direction)
local newDir
local right = {N="E", E="S", S="W",W="N"}
if lOrR == "right" then
return right[direction]
end
end
The newDir isn’t used. And let’s have nested data:
function World:getNewDirection(lOrR, direction)
local lookup = {
right = {N="E", E="S", S="W",W="N"},
left = {N="W", W="S", S="E", E="N"}
}
return lookup[lOrR][direction]
end
We’re green. Commit: Can get new direction for left or right turn.
Now the flaw. The method above is clearly not really much of a World method, as it references nothing about the world. I suppose we could argue that it is a fact of the world that if you turn right while heading north, you’ll wind up going east.
I don’t see an improvement. Let’s make turning take you to the right point. We need some more tests:
_:test("turns take you to the right places", function()
local world = World(25,25)
local rq = {
robot="xyzzy",
command="launch",
arguments={"plugh", 99,99}
}
world:request(rq)
local robot = world:getRobot("xyzzy")
world:forward("xyzzy", 2)
_:expect(robot._x).is(0)
_:expect(robot._y).is(2)
end)
This runs green. Extend it a bit:
world:forward("xyzzy", 2)
_:expect(robot._x).is(0)
_:expect(robot._y).is(2)
world:turn("xyzzy","right")
world:forward("xyzzy", 3)
_:expect(robot._x).is(3)
_:expect(robot._y).is(2)
This’ll fail with a 5 for y and nothing for x.
23: turns take you to the right places --
Actual: 0,
Expected: 3
23: turns take you to the right places --
Actual: 5,
Expected: 2
As expected. Enhance turn … no, forward. Here’s the current code:
function World:back(robotName,steps)
return self:moveFB(robotName, -1, steps)
end
function World:forward(robotName,steps)
return self:moveFB(robotName, 1, steps)
end
function World:moveFB(robotName,step, steps)
local robot = self._robots[robotName]
robot._y = robot._y + step*steps
local response = {
result = "OK",
data = {},
state = self:robotState(robot)
}
return response
end
Clearly we need to adjust both x and y, depending on the direction we’re facing. Small steps, no pun intended.
Add a local variable for stepping y.
function World:moveFB(robotName,step, steps)
local yStep = 1
local robot = self._robots[robotName]
robot._y = robot._y + yStep*step*steps
local response = {
result = "OK",
data = {},
state = self:robotState(robot)
}
return response
end
Tests should be the same. They are. Add a local variable for stepping x:
function World:moveFB(robotName,step, steps)
local xStep = 0
local yStep = 1
local robot = self._robots[robotName]
robot._x = robot._x + xStep*step*steps
robot._y = robot._y + yStep*step*steps
local response = {
result = "OK",
data = {},
state = self:robotState(robot)
}
return response
end
Tests are the same. Fold the two inits into one:
local xStep,yStep = 0,1
Tests are the same. Change the right side to a table, and unpack it:
local xStep,yStep = table.unpack({0,1})
Tests the same. Call a method to get the table:
function World:moveFB(robotName,step, steps)
local xStep,yStep = self:getSteps()
local robot = self._robots[robotName]
robot._x = robot._x + xStep*step*steps
robot._y = robot._y + yStep*step*steps
local response = {
result = "OK",
data = {},
state = self:robotState(robot)
}
return response
end
function World:getSteps()
return table.unpack({0,1})
end
Tests are the same. Implement the method correctly:
function World:getSteps()
local steps = {
N={0,1}, E={1,0}, S={0,-1},W={-1,0}
}
return table.unpack(steps[robot._direction])
end
Realize we need a robot or a direction and have neither. Take a direction:
function World:getSteps(direction)
local steps = {
N={0,1}, E={1,0}, S={0,-1},W={-1,0}
}
return table.unpack(steps[direction])
end
Change the caller:
function World:moveFB(robotName,step, steps)
local robot = self._robots[robotName]
local xStep,yStep = self:getSteps(robot._direction)
robot._x = robot._x + xStep*step*steps
robot._y = robot._y + yStep*step*steps
local response = {
result = "OK",
data = {},
state = self:robotState(robot)
}
return response
end
Expect the test to pass. It does.
We’d best extend the test:
_:test("turns take you to the right places", function()
local world = World(25,25)
local rq = {
robot="xyzzy",
command="launch",
arguments={"plugh", 99,99}
}
world:request(rq)
local robot = world:getRobot("xyzzy")
world:forward("xyzzy", 2)
_:expect(robot._x).is(0)
_:expect(robot._y).is(2)
world:turn("xyzzy","right") -- east
world:forward("xyzzy", 3)
_:expect(robot._x).is(3)
_:expect(robot._y).is(2)
world:turn("xyzzy", "right") -- south
world:forward("xyzzy",5)
_:expect(robot._x).is(3)
_:expect(robot._y).is(-3)
world:turn("xyzzy","right") -- west
world:forward("xyzzy",3)
_:expect(robot._x).is(0)
_:expect(robot._y).is(-3)
end)
Green. Commit: World handles turns left and right, moves go in correct direction.
Moving in the game does not yet move us in the direction we’d expect. I think that’s because we aren’t talking to the world about it.
Hook Up Robot Turn.
function Robot:keyboard(key)
if key == "l" then
self:scan() -- look
elseif key == "1" then
self:forward(1)
elseif key == "w" then
self:forward(1)
elseif key == "s" then
self:back(1)
elseif key == "r" then
local lens = RotatingLens(self.knowledge)
lens:turn("right")
self.knowledge = lens
end
end
We just used the r key to turn right and used the RotatingLens, mostly to test it. Now we need to refer this command to the world:
function Robot:keyboard(key)
if key == "l" then
self:scan() -- look
elseif key == "1" then
self:forward(1)
elseif key == "w" then
self:forward(1)
elseif key == "s" then
self:back(1)
elseif key == "r" then
self:turn("right")
end
end
function Robot:turn(lOrR)
self:setResponse(self._world:turn(self._name,lOrR))
self.knowledge = self.knowledge:newLensAt(self:x(),self:y())
end
I’m doing this without a net. Very bad. Stop, write a test.
_:test("robot moves correctly after turn", function()
local world = WorldProxy()
local robot = Robot("Wilbur", world)
_:expect(robot:x()).is(0)
_:expect(robot:y()).is(0)
robot:turn("right")
robot:forward(2)
_:expect(robot:x()).is(2)
_:expect(robot:y()).is(0)
end)
9: robot moves correctly after turn -- TestRobot:181: attempt to call a nil value (method 'turn')
That’ll be the missing one in proxy.
function World:turn(name, lOrR)
local rq = {
name=name,
command="turn",
arguments = { lOrR }
}
return Response(self.world:request(rq))
end
I definitely feel rattled. Steps are too big. Lots of tests break.
22: turn right changes direction --
Actual: ERROR,
Expected: OK
22: turn right changes direction --
Actual: SERVER ERROR TestWorldProxy:94: attempt to index a nil value (field 'world'),
Expected:
22: turn right changes direction --
Actual: N,
Expected: E
22: turn right changes direction -- TestWorld:526: attempt to index a nil value (field 'state')
23: turns take you to the right places -- TestWorldProxy:94: attempt to index a nil value (field 'world')
9: robot moves correctly after turn -- TestRobot:181: attempt to call a nil value (method 'turn')
This may not be as bad as it seems. Then again, it may be. Well it would have helped if I’d put that method on WorldProxy:
function WorldProxy:turn(name, lOrR)
local rq = {
name=name,
command="turn",
arguments = { lOrR }
}
return Response(self.world:request(rq))
end
A reasonable compiler would have told me about that. Test again hoping for less chaos.
Bad Result : ERROR SERVER ERROR
TestWorld:280: attempt to index a nil value (local 'robot')
9: robot moves correctly after turn --
TestResponse:61: attempt to index a nil value (field 'state')
OK, what’s happening in World?
function World:turn(robotName, lOrR)
local robot = self:getRobot(robotName)
robot._direction = self:getNewDirection(lOrR, robot._direction)
local response = {
result="OK",
data = {},
state=self:robotState(robot)
}
return response
end
We didn’t find our robot. Why not? Enhance that method:
local robot = self:getRobot(robotName)
assert(robot,"Cannot find robot "..tostring(robotName))
Test.
Bad Result : ERROR SERVER ERROR TestWorld:280: Cannot find robot nil
We didn’t pass in his name to turn … or the request had a nil name …
Add checking …
Find bad request format, should be:
function WorldProxy:turn(name, lOrR)
local rq = {
robot=name,
command="turn",
arguments = { lOrR }
}
return Response(self.world:request(rq))
end
I had said name=, not robot=. Tests are green.
We have turn working end to end. Commit: turn works end to end. only keyboard command is “r” so far.
Let’s sum up.
Summary
Weird. Things got more ragged as the day wore on. I felt things were quite smooth and steps were small at the beginning, and at the end, though the code was jusst about right (only the incorrect name), I had just about entirely lost the thread of what I was up to.
That last commit changed three files, Robot, World and WorldProxy, and the World ones were trivial. In Robot I just added a test and forwarded to the proxy, and the proxy code is only 9 trivial lines (with one error).
Maybe I got tired or distracted. I started at 0930, and it’s 1315 now, so that’s a bit long for half a banana and a sip or two of chai. I was really heads down and moving too fast, I think.
Slow and steady gives me time to get what I’m doing firmly embedded in my mind. The changes I needed were all “obvious” and I typed them in without really thinking. Probably not the best way to operate.
On the bright side, we refactored some code to a much better state, removing big chunks of duplication, and we have the game now allowing you to turn and move around on the map. We have forward and back working, and they support multiple steps, even though there’s no way to request such things.
We wrote new tests and our old tests showed their value during refactoring. I feel pretty good about the testing quality here.
So … a good day, but I still need to work on smaller steps, and maybe more intentional breaks.
See you next time!