Simple maze game with four rooms

I'm starting to learn Lua and in one part of the book I need to convert a "Maze Game" that uses goto to another one that doesn't use goto.

The maze is the simplest ever made, starting from room #1:

---------
| 1 | 2 |
---------
| 3 | 4 |
---------


This is the original version using goto from the book:

goto room1 -- initial room
::room1:: do
if move == "south" then goto room3
elseif move == "east" then goto room2
else
print("invalid move")
goto room1 -- stay in the same room
end
end

::room2:: do
if move == "south" then goto room4
elseif move == "west" then goto room1
else
print("invalid move")
goto room2
end
end

::room3:: do
if move == "north" then goto room1
elseif move == "east" then goto room4
else
print("invalid move")
goto room3
end
end

::room4:: do
print("Congratulations, you won!")
end


And this is my attempt:

local rooms = {}
rooms[1] = {
south=3,
east=2
}
rooms[2] = {
south=4,
west=1
}
rooms[3] = {
north=1,
east=4
}
currentRoom = 1
repeat
local room = rooms[currentRoom][move]
if room == nil then
room = currentRoom
print("Invalid move")
elseif room == 4 then
print("Congratulations, you won!")
end
currentRoom = room or currentRoom
until currentRoom == 4


The above code is working but I was wondering if there is something I need to change to improve it somehow.

• Have you tested your attempt? Does your attempt work? – Simon Forsberg Mar 22 '16 at 17:09
• @SimonForsberg hi, yest it does work. I'll update my question – César Mar 22 '16 at 17:10

I think overall is good, some bits can be improved but it depends on taste. Take the following comments as suggestions.

As rooms is an array, can be defined in a shorter way as:

local rooms = {
{ south=3, east=2 },
{ south=4, west=1 },
{ north=1, east=4 },
}


Variable currentRoom can be localized:

local currentRoom = 1


In Lua can compare a var to nil or ask not var:

if not room then
...
end


Branches are exclusive, use of elseif can be skipped:

if not room then
room = currentRoom
print("Invalid move")
end
if room == 4 then
print("Congratulations, you won!")
end


EDIT:

I would rearrange the code at the bottom as:

-- Update room state.
room = rooms[currentRoom][move]
if room then
currentRoom = room
else
print("Invalid move")
end

-- Reached final state?
if currentRoom == 4 then
print("Congratulations, you won!")
end


that avoids the currentRoom = room or currentRoom statement (currentRoom only gets updated is room is not nil).

• Welcome to Code Review! Good job on your first answer. – SirPython Apr 13 '16 at 23:52
• Thanks @SirPython. I hope I can engage more and keep learning and improving my skills while reviewing code. – Diego Pino Apr 14 '16 at 7:03
• @DiegoPino Is there a reason why you prefer omitting elseif? IMO elseif is shorter and more importantly makes immediately clear to the reader that only one of the branches will be taken. Intuitively I'd also assume elseif would be more performant because if room is nil, the second check can be skipped. Not that performance would matter in this specific case, but as I see three arguments (brevity, clarity, performance) in favor of elseif and no obvious one for two ifs, I'd argue making elseif a habit seems like a good idea. – Thomas W. Apr 20 '16 at 5:37
• @ThomasW. I think those are good points. My rule of thumb is to use elseif where a switch statement would fit. I think switch is mostly helpful when there are several branches, more than two. In this case I wouldn't use it, but that's mostly a personal preference. – Diego Pino Apr 20 '16 at 7:29

I see many small mistakes that can be improved upon, Diego Pino did an excellent job of pointing out a few, I'll include his and explain the ones I add in myself:

local rooms = {{south=3,east=2},{south=4,west=1},{north=1,east=4}}
local currentRoom = 1
local room --Room is used every interaction, so I declared it out here
while true do --Changed loop for performance reasons
room = rooms[currentRoom][io.read()] --Move variable was only used once so I replaced it
if not room then
room = currentRoom
print("Invalid move")
elseif room == 4 then
print("Congratulations, you won!")
break --I break here rather than doing an additional check at the end of our loop iteration
else
currentRoom = room --Put this inside and else since it's only necessary if another room is picked
end
end