# Messed up Elevator Management System

In light of our current I decided to build an Elevator Management System.

Initially I intended to program the EMS like a real-time operating system and the elevators as finite-state machine threads with each their own handler. It didn't go according to plan, but it works. For some values of works, since my current infrastructure makes writing tests quite hard.

Thread safety is quite an issue here. I'm grasping this opportunity to learn a thing or two about multi-threading and best practices surrounding that topic.

There's also a generalization problem. As in, there's way too much copy-paste going on. I would fix that, but since I think the architectural flaws are a much bigger problem than copied code.

This project is not finished. However, I think the current state is perfectly reviewable since it's doing what it's supposed to do.

Smaller issues:

• broadcast_event is global while it should be a member of Bank. Putting it in there bricked the function, probably due to another design flaw.
• I'm using sys.stdout.write instead of print because print is not thread-safe. However, this feels like an ugly hack.
• I should probably split up the code to split the module from the invocations. This is planned after the broadcast_event issue mentioned earlier is fixed.
• The elevator.target attribute is unused. This should be the final destination of the elevator before it turns IDLE again, while still stopping at all floors in between.
• The elevators are unfamiliar with the concept of doors. Same goes for time and size.
• The message handling of the elevators should probably be split from the actual movement.
• No documentation yet. This is a known problem and will be addressed in the next version.

import sys
import Queue
import time

banks_mail = []
elevators_mail = []
elevators = []
floors_mail = []

def __init__(self):
self.mailbox = Queue.Queue()
banks_mail.append(self.mailbox)

def run(self):
while True:
data = self.mailbox.get()
sys.stdout.write(str(self) + ' received: ' + str(data) + '\n')
if data == 'shutdown':
sys.stdout.write(str(self) + ' shutting down' + '\n')
return
elif data[1] == 'UP':
for idx, elevator in enumerate(elevators):
if elevator.floor <= data[0] and (elevator.direction == 'UP' or elevator.direction == 'IDLE'):
sys.stdout.write('Elevator ' + str(idx) + ' available for floor ' + str(data[0]) + '\n')
elevators_mail[idx].put(data)
break
elif data[1] == 'DOWN':
for idx, elevator in enumerate(elevators):
if elevator.floor >= data[0] and (elevator.direction == 'DOWN' or elevator.direction == 'IDLE'):
sys.stdout.write('Elevator ' + str(idx) + ' available for floor ' + str(data[0]) + '\n')
elevators_mail[idx].put(data)
break
else:

def stop(self):
banks_mail.remove(self.mailbox)
self.mailbox.put('shutdown')
self.join()

def __init__(self, floor=0, direction='IDLE'):
self.mailbox = Queue.Queue()
elevators_mail.append(self.mailbox)
elevators.append(self)
self.floor = floor
self.direction = direction
self.target = self.floor

def run(self):
while True:
data = self.mailbox.get()
sys.stdout.write(str(self) + ' received: ' + str(data) + '\n')
sys.stdout.write(str(data))
if data == 'shutdown':
sys.stdout.write(str(self) + ' shutting down' + '\n')
return
elif data[0] == self.floor:
sys.stdout.write(str(self) + ' already at ' + str(data[0]) + '\n')
break
elif data[0] >= self.floor:
self.direction = 'UP'
for i in xrange(self.floor, data[0]):
sys.stdout.write(str(self.floor) + ' from/to ' + str(data[0]) + '\n')
self.floor += 1
sys.stdout.write(str(self) + ' at ' + str(self.floor) + ' going up\n')
self.direction = 'IDLE'
elif data[0] <= self.floor:
self.direction = 'DOWN'
for i in xrange(data[0], self.floor):
sys.stdout.write(str(self.floor) + ' from/to ' + str(data[0]) + '\n')
self.floor -= 1
sys.stdout.write(str(self) + ' at ' + str(self.floor) + ' going down' + '\n')
self.direction = 'IDLE'

def stop(self):
elevators_mail.remove(self.mailbox)
self.mailbox.put('shutdown')
self.join()

def __init__(self, number=0):
self.mailbox = Queue.Queue()
floors_mail.append(self.mailbox)
self.number = number

def run(self):
while True:
data = self.mailbox.get()
if data == 'shutdown':
sys.stdout.write(str(self) + ' shutting down' + '\n')
return
sys.stdout.write(str(self) + ' received: ' + str(data) + '\n')

def stop(self):
floors_mail.remove(self.mailbox)
self.mailbox.put('shutdown')
self.join()

def call(self, data):
banks_mail[0].put((self.number, data))

for q in elevators_mail:
q.put(data)

b0 = Bank()
b0.start()

t1 = Elevator()
t2 = Elevator()
t3 = Elevator(8, 'DOWN')
t1.start()
t2.start()
t3.start()

f0 = Floor(0)
f1 = Floor(1)
f2 = Floor(2)
f3 = Floor(3)
f4 = Floor(4)
f5 = Floor(5)
f6 = Floor(6)
f7 = Floor(7)
f8 = Floor(8)

f0.start()
f1.start()
f2.start()
f3.start()
f4.start()
f5.start()
f6.start()
f7.start()
f8.start()

f1.call('UP')
f4.call('UP')
f6.call('DOWN')

time.sleep(0.5) # Make sure all other calls are finished before shutdown

sys.stdout.write('\nRecall all elevators back to Floor 0\n\n')

f0.stop()
f1.stop()
f2.stop()
f3.stop()
f4.stop()
f5.stop()
f6.stop()
f7.stop()
f8.stop()

t1.stop()
t2.stop()
t3.stop()

b0.stop()

• To quote RubberDuck: So, the elevator challenge is a lot harder than I expected.
– Mast
Sep 27 '15 at 16:53

There's also a generalization problem.

I agree! Whenever you start writing things like:

f0 = Floor(0)
f1 = Floor(1)
f2 = Floor(2)
f3 = Floor(3)
f4 = Floor(4)
f5 = Floor(5)
f6 = Floor(6)
f7 = Floor(7)
f8 = Floor(8)


stop immediately, and consider using lists and/or dictionaries instead:

floors = [Floor(i) for i in range(9)]


This now makes subsequent repetition easier to factor out, e.g.:

for floor in floors:
floor.start()


and if you add a new Floor, you don't have to change much of the code. It's a bit trickier with the elevators, because some of them use default arguments, but you could do something like:

elevators = [Elevator(*args) for args in ((), (), (8, 'DOWN'))]


You can do something similar to replace:

f1.call('UP')
f4.call('UP')
f6.call('DOWN')


with e.g.

for floor, direction in ((1, 'UP'), (4, 'UP'), (6, 'DOWN')):
floors[floor].call(direction)


One clear structural problem is the global variables. Imagine adding another Bank to the script - what happens then? Clearly elevators in two different banks cannot serve the same floors, which suggests that e.g. the list of elevators should live within the Bank instance in which the elevators sit.

I would also be tempted to allow a more direct sending of messages. Rather than:

elevators_mail[idx].put(data)


why not simply:

elevator.call(data)


Each Elevator instance then exposes its own mailbox via appropriate methods, rather than having a separate structure storing them.

• Looks like I overlooked quite some things which are relatively easy to implement and still have a major impact. Thanks!
– Mast
Sep 27 '15 at 17:42