Simple PIN Locker in Python

I am fairly new to Python but I have been improving, and I wanted to try writing a simple program that simulates locker with multiple chambers (4) for storing "objects".
I recently used lockers at a theme park with a similar functionality.

A chamber can be reserved by simply typing a unique PIN. In the real world, the PIN would be a unique ID like a credit card track, a finger print, a barcode, or RFID.

If the PIN is already associated with a chamber, the chamber is released and made available and the user is destroyed.

If the PIN is unique, and there are chambers available (not reserved),
a chamber is reserved for the user under the unique PIN.

I hope the explanation makes sense.
I am looking for some pointers and critique on code organization and flow, best practices, and use of classes, methods, etc.

You can see the code in action here.

```
class Locker(object):

def __init__(self, capacity):
self.chambers = {}

print 'Locker Initialized.'
for n, chamber in enumerate(range(0, capacity)):
self.chambers[n] = Chamber(n)
print 'Locker Created: ', self

def find_chamber_by_pin(self, pin):
for chamber in self.chambers.values():
if pin == getattr(chamber.user, 'pin', None):
return chamber
return None

def find_available_chamber(self):
for chamber in self.chambers.values():
if chamber.occupied is False:
return chamber
return None

def count_available(self):
counter = 0
for chamber in self.chambers.values():
if chamber.occupied is False:
counter += 1
return counter

def __len__(self):
return len(self.chambers.keys())

def __repr__(self):
return ':'.format(self.count_available(), len(self))

class Chamber(object):

def __init__(self, uid):
self.uid = uid
self.occupied = False
self.user = None
print 'Chamber Init

Though I've got a lot to say, it's mostly just me pulling out stuff from my Python toybox, which let you write what you want, rather than how to get it. In general, this is a really clean module and there's nothing utterly heinous going on in there.

def __init__(self, capacity):
    self.chambers = {}

    print 'Locker Initialized.'
    for n, chamber in enumerate(range(0, capacity)):
        self.chambers[n] = Chamber(n)
    print 'Locker Created: ', self

For starters, you don't need to use a dict for this, a list will do just fine. You're giving yourself a constant number of chambers, nothing new ever enters, and nothing ever gets deleted. You may have done this because you've got a C or Java background, know about linked lists, and are a bit worried about access times. In Python, a "list" is actually an array under the hood, so you actually gain by using one.

The range function by default goes from 0 to n-1, so adding a 0 parameter is needless here.

Enumerate will zip together an enumerable with incrementing integers. Range returns a list of incrementing integers, so calling enumerate(range(0,n)) gives you back [(0,0), (1,1), (2,2), ..., (n-1,n-1)] (effectively, it's a little subtler than that). I can't really think of a use case for this, and you don't actually use the second part of each tuple anyway, so you can strip that out.

Finally, comprehensions are a really useful way of generating lists more cleanly. Rather than having a loop that keeps appending things, describing how a result is achieved, you instead write an expression that states what the result is, which makes your intention a little clearer. I'd replace your __init__ with

def __init__(self, capacity):
    print 'Locker Initialized.'        
    self.chambers = [Chamber(n) for n in range(capacity)]
    print 'Locker Created: ', self

def find_chamber_by_pin(self, pin):
    for chamber in self.chambers.values():
        if pin == getattr(chamber.user, 'pin', None):
            return chamber
    return None

getattr is awesome, but when you use it with a constant string you don't gain a great deal. You know that chamber.user will always be either an instance of User or None, and in Python None is falsey, so you can replace your condition with if chamber.user and pin == chamber.user.pin:

I'd go a little further than this, though. Your loop is over the list of occupied chambers, so let's call it that...

def occupied_chambers(self):
    return (chamber for chamber in self.chambers if self.chamber.occupied)

def find_chamber_by_pin(self, pin):
    for chamber in self.occupied_chambers():
        if chamber.user.pin == pin:
            return chamber
    return None

If you wanted to crush your number of lines down, you could also exploit next, but the resulting function starts to look a little chaotic. It's there if you like it:

def find_chamber_by_pin(self, pin):
    return next((chamber for chamber in self.occupied_chambers() 
                     if chamber.user.pin == pin), None)

def find_available_chamber(self):
    for chamber in self.chambers.values():
        if chamber.occupied is False:
            return chamber
    return None

def count_available(self):
    counter = 0
    for chamber in self.chambers.values():
        if chamber.occupied is False:
            counter += 1
    return counter

There aren't all that many situations where you gain from comparing stuff to True and False. Rather than chamber.occupied is False, not chamber.occupied is preferred (but see later on).

These functions both iterate over the same thing, so abstract it out:

def available_chambers(self):
    return [chamber for chamber in self.chambers if not chamber.occupied]

def find_available_chamber(self):
    try:
        return self.available_chambers()[0]
    except IndexError:
        return None

def count_available(self):
    return len(self.available_chambers())

class Chamber(object):
    def __init__(self, uid):
        self.uid = uid
        self.occupied = False
        self.user = None
        print 'Chamber Initialized: ', self

    def reserve(self, user):
        self.occupied = True
        self.user = user
        print 'Chamber Reserved: ', self

    def release(self):
        self.occupied = False
        user = self.user
        del user
        self.user = None
        print 'Chamber Released: ', self

First off, your release method deletes its user. This doesn't actually achieve anything. You can just set the user to None and the garbage collector will sort it out for you when it notices your object no longer references it.

Second, it seems like you want to keep an invariant that occupied is True if user is set and occupied is false if user is unset. For something like this, I'd use the property decorator which, among other things, lets you create properties that only exist based on other properties of your object. H

StackExchange Code Review Q#120372, answer score: 2