Parking Lot OO Design (Python)

Question

Please review my code for parking lot design. I am new to OOD concepts, It will be great to have some feedback on OO Structure of the solution.

Requirements considered:

1. The parking lot can have multiple levels.
2. Each level can have 'compact', 'large', 'bike' and 'electric' spots.
3. Vehicle should be charged according to spot type, time of parking and duration of parking.
4. Should be able to add more spots to level.
5. Show the available number of spots on each level.

class parkingFloor():
 def __init__(self,name):
 self.name = name
 self.spotTotal = {'compact':0,'large':0,'bike':0,'electric':0}
 self.spotTaken = {'compact':0,'large':0,'bike':0,'electric':0}
 self.freeSpot = {'compact':set(),'large':set(),'bike':set(),'electric':set()}
 self.takenSpot = {'compact':{},'large':{},'bike':{},'electric':{}}
 def assignSpot(self,tickt):
 if self.spotTaken[tickt.veh.type] >= self.spotTotal[tickt.veh.type]:
 return False
 for s in self.freeSpot[tickt.veh.type]:
 if s.id not in self.takenSpot[tickt.veh.type]:
 self.takenSpot[tickt.veh.type][s.id] = tickt
 self.spotTaken[tickt.veh.type]+=1
 self.freeSpot[tickt.veh.type].remove(s)
 tickt.allocateSpot(s)
 return True
 return False
 def addSpot(self,type,v):
 for i in range(v):
 s = spot(type)
 self.freeSpot[type].add(s)
 self.spotTotal[type] += v
class entryPanel():
 def __init__(self,id):
 self.id = id
 def printTicket(self,tickt):
 print('Vehicle ID ',tickt.veh.id)
 print('Spot ID ',tickt.spot.id)
 print('Ticket ID ',tickt.id)
 print('Date Time',tickt.DateTime)
 def display(self,message):
 print(message)
class vehicle():
 def __init__(self,id,vehType):
 self.id = id
 self.type = vehType
class spot():
 def __init__(self,spotType):
 def generateId():
 # some mechanism to generate spot id
 return 1
 self.id = generateId()
 self.type = spotType
class ticket():
 def __init__(self,v1):
 self.id = self.generateId()
 self.veh = v1
 self.spot = None
 self.DateTime = self.getTime()
 self.amount = 0
 self.status = 'Active'
 self.payment = None
 def getTime(self):
 time = 1234
 return time
 def generateId(self):
 # some mechanism to generate new ticket id
 new_ticket = 1
 return new_ticket
 def allocateSpot(self,spot):
 self.spot = spot
 def addPayment(self,pay):
 self.status = 'Complete'
 self.payment = pay
class parkingLot():
 def __init__(self,name,address):
 self.name = name
 self.address = address
 self.level = []
 def addLevel(self,floor):
 self.level.append(floor)
 def processEntry(self,t1,gate):
 for l in self.level:
 if l.assignSpot(t1):
 gate.printTicket(t1)
 return
 gate.display('No Spot Empty')
 def processExit(self,tickt,gate):
 def getTime():
 # Gives the current time
 return 3
 currTime = getTime()
 print('Processing fare',tickt.veh.type,tickt.spot.id,tickt.DateTime,currTime)
 amountCalculated = 7
 tickt.addPayment(Payment(amountCalculated))
 gate.display('Payment Successful')
class Payment():
 def __init__(self,amount):
 self.id = 'paymentid2'
 self.type = 'credit' # debit
 self.time = 'paymet time'
class displayBoard():
 def show(self,p):
 for l in p.level:
 print(l.name)
 for k in l.spotTotal.keys():
 print(k, l.spotTotal[k] - l.spotTaken[k])
P = parkingLot('Savita','Address')
floor1 = parkingFloor('floor1')
P.addLevel(floor1)
floor1.addSpot('compact',5)
board = displayBoard()
board.show(P)
entryPanel1 = entryPanel('1')
v1 = vehicle(1,'compact')
t1 = ticket(v1)
P.processEntry(t1,entryPanel1)
P.processExit(t1,entryPanel1)

• AlexV: Welcome to Code Review! You've posted a lot of code to review. Consider giving the reviewers a hint where the focus of the review should be (e.g. style, clarity, OO structure, ...). Also elaborate on what the expected outcome of the provided test code should be.

Answer 1

Overall, this is actually very good, especially considering you're new to the OOP design pattern. There're just a few things I would recommend changing:

Naming conventions

There's a list of naming conventions that are widely accepted all throughout the python community. Following these conventions makes it easier to share and develop your code with other developers because everyone's already familiar with the conventions. It also makes your life easier because you have a set of guidelines to follow, and you're more likely to keep your conventions consistent, thus making your code easier to understand.

Your class names don't follow the convention of naming classes in CamelCase starting with a capital letter. So you should change parkingFloor to ParkingFloor, entryPanel to EntryPanel, etc.

Enums over strings

In several places, you're using strings where you should be using enums. Enums are basically special classes filled with constants and are preferable in certain situations when you're using the same strings in multiple places. Python does have built-in support for enums, and you should use it.

Your whole ticket class could benefit largely from this. My suggestion is to create a class called TicketStatus extending enum.Enum (thus making it an enum), and create within it the constants ACTIVE and COMPLETE. You can then change these two statements:

self.status = 'Active'
self.status = 'Complete'

to this:

self.status = TicketStatus.ACTIVE
self.status = TicketStatus.COMPLETE

Using enums makes comparisons and debugging easier, and overall makes your code a bit easier to understand.

Comments/documentation

Looking through your entire code, you have only 4 comments, 3 of which are placeholders for code you have not yet written. This is not good documentation. I can guarantee that if you stop working on your project and come back a month later, you will wish you explained to yourself what your code does and why. Or, if another developer starts working on your project, they won't know what much of the code does.

In general, you should document almost everything. This makes it easier for both yourself and others who are viewing your code. Docstrings (you may have heard them called "multiline comments") should be used when documenting a class, function, module, or method.

Inline comments should be used a little more sparingly. If anyone who has never seen your program before can easily tell what a piece of code does, you do not need to add a comment explaining it. But if the purpose of the statement is not entirely obvious, adding a comment can be useful.

Answer 2

Pikachu the Purple Wizard's answer covers quite a lot of essential style hints and best practices such as using enums.

However, he has not said a lot about the actual classes themselves. Especially displayBoard caught me attention immediately.

class displayBoard():
 def show(self,p):
 for l in p.level:
 print(l.name)
 for k in l.spotTotal.keys():
 print(k, l.spotTotal[k] - l.spotTaken[k])

This is kind of a nonsense class. Classes should generally be used to hold variables and methods that would naturally belong together. displayBoard does not hold any data, and has a single function. Unlike Java, Python allows for functions which are not part of a class. So this could easily become

def displayBoard(p):
 for l in p.level:
 print(l.name)
 for k in l.spotTotal.keys():
 print(k, l.spotTotal[k] - l.spotTaken[k])

without losing any functionality, clearity or whatever. As Pickachu said, this function would greatly benefit from documentation, and more expressive variable names (What type of object should p be?).

On the other hand, Payment is as a fully valid candidate for a class. However your code ignores the only input value one might pass to its constructor.

class Payment():
 def __init__(self, amount): #<- amount is not used anywhere
 self.id = 'paymentid2' #<- all these values may only be set afterwards, why?
 self.type = 'credit' # debit
 self.time = 'payment time'

While reworking your code, also think about whether a string is really a good fit for storing time. E.g. Python has a built-in datetime module which might be (and probably is) a better fit here.

Another thing sou should change are those nested functions sometimes found in methods or in the constructor of some classes.

class spot():
 def __init__(self,spotType):
 def generateId():
 # some mechanism to generate spot id
 return 1
 self.id = generateId()
 self.type = spotType

Should be reworked into

class Spot():
 def __init__(self, spotType):
 self.id = self.generateId()
 self.type = spotType
 def generateId(self):
 """some mechanism to generate spot id"""
 return 1

which is what you already do, e.g. for class ticket(). If you intend to show that generateId(self) is not supposed to be used outside of the class, prepend it with _. That does not really hide the function from users outside of your class, but it is generally accepted as a convention that this function is only for internal use only and might be changed/removed/... without further notice.

Answer 3

Working code for the above questions -

1. Better naming conventions
2. Used enums over strings

A few more changes are there since I added some more functionality like -

1. Car / Bike classes extend the Vehicle Base Class
2. Ticket has Vehicle, Spot and Payment objects
3. Have added unassignSpot() as well

Other people are welcome to review my code and provide your valuable inputs. Thank you.

import time
from enum import Enum
class TicketStatus (enum.Enum) :
 ACTIVE = 1
 COMPLETE = 2
class VehicleType (enum.Enum) :
 CAR = 1
 BIKE = 2
class SpotType (enum.Enum) :
 FREE = 1
 TAKEN = 2
class ParkingLot :
 def __init__(self, name, address) :
 self.name = name
 self.address = address
 self.level = []
 def addLevel(self, floor):
 self.level.append(floor)
 def processEntry(self, ticket) :
 for eachlevel in self.level :
 if eachlevel.spots[ticket.veh.type][SpotType.FREE] :
 ticket.spot = eachlevel.assignSpot(ticket)
 print('Entry Completed For : ', ticket.veh.num)
 break;
 def processExit(self, ticket) :
 for eachlevel in self.level :
 if ticket.spot in eachlevel.spots[ticket.veh.type][SpotType.TAKEN] :
 eachlevel.unassignSpot(ticket)
 break;
 ticket.outTime = time.time()
 ticket.spots = None
 ticket.status = TicketStatus.COMPLETE
 ticket.payment = Payment(ticket.inTime, ticket.outTime)
 print('Exit Completed For : ', ticket.veh.num, ' Pay : ', int(ticket.payment.amount))
class ParkingLevel :
 def __init__(self, name) :
 self.name = name
 self.spots = {VehicleType.CAR : {SpotType.FREE : [], SpotType.TAKEN : []}, 
 VehicleType.BIKE : {SpotType.FREE : [], SpotType.TAKEN : []} }
 def assignSpot(self, ticket) :
 if self.spots[ticket.veh.type][SpotType.FREE] != [] :
 spot = self.spots[ticket.veh.type][SpotType.FREE].pop()
 ticket.spot = spot
 self.spots[ticket.veh.type][SpotType.TAKEN].append(spot)
 return ticket.spot
 return False
 def unassignSpot(self, ticket) :
 self.spots[ticket.veh.type][SpotType.FREE].append(ticket.spot)
 self.spots[ticket.veh.type][SpotType.TAKEN].remove(ticket.spot)
 def addSpot(self, type, num) :
 for eachnum in range(num) :
 spot = Spot(type)
 self.spots[type][SpotType.FREE].append(spot) 
class Vehicle :
 def __init__(self, num) :
 self.id = self.generateID()
 self.num = num
 def generateID(self) :
 yield range(100)
class Car (Vehicle) :
 def __init__(self, num) :
 super().__init__(num)
 self.type = VehicleType.CAR
class Bike (Vehicle) :
 def __init__(self, num) :
 super().__init__(num)
 self.type = VehicleType.BIKE
class Spot :
 def __init__(self, type) :
 self.id = self.generateID()
 self.type = type
 def generateID(self) :
 yield range(100)
class Payment :
 def __init__(self, inTime, outTime) :
 self.mode = None
 self.rate = [30, 20, 10]
 self.amount = self.calAmount(inTime, outTime)
 def getRate(self) :
 return self.rate
 def setRate(self, rate) :
 self.rate = rate
 def calAmount(self, inTime, outTime) :
 amount = (outTime - inTime) * self.getRate()[0]
 amount += (outTime - inTime - 60 ) * self.getRate()[1] if outTime - inTime - 60 > 0 else 0
 amount += (outTime - inTime - 120 ) * self.getRate()[2] if outTime - inTime - 120 > 0 else 0
 return amount 
class Ticket :
 def __init__(self, veh) :
 self.veh = veh
 self.status = TicketStatus.ACTIVE
 self.inTime = time.time()
 self.outTime = None
 self.payment = None
 self.spot = None
 def generateID(self) :
 # some ID generation mechanism
 return ID
class DisplayBoard :
 def show(self, P) :
 for eachlevel in P.level :
 print(P.name , '-' , eachlevel.name, '- Available Parking Spots')
 print('Car : ', len(eachlevel.spots[VehicleType.CAR][SpotType.FREE]))
 print('Bike : ', len(eachlevel.spots[VehicleType.BIKE][SpotType.FREE]))
P = ParkingLot('Google Parking Lot', '123, Fort, Mumbai')
F1 = ParkingLevel('F1')
F1.addSpot(VehicleType.CAR, 3)
F1.addSpot(VehicleType.BIKE, 3)
P.addLevel(F1)
Board = DisplayBoard()
Board.show(P)
T1 = Ticket(Car('MH05 AB 5454'))
P.processEntry(T1)
T2 = Ticket(Bike('MH05 AB 9000'))
P.processEntry(T2)
time.sleep(2) 
P.processExit(T2)
Board = DisplayBoard()
Board.show(P)

• Sᴀᴍ Onᴇᴌᴀ: If you want this code reviewed then you should ask a new question and include a link to the question above instead of putting it in an answer.
• pratikjain227: I appreciate your views, my comment was an answer to the question posted here. And I also welcome other's feedback on my comment, if any.