HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

Flight booking system

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
bookingflightsystem

Problem

I'm attempting to improve my OOP design skills through solving sample problems. Below is an implementation of a flight booking system where the passenger can only request a seat class, but no number. Also, flight statistics must be printed.

In your review, please focus on design: How can I design the classes better (or design better classes) to lower coupling, increase cohesion and maintainability?

One source of 'developer anxiety' I get is when I send arguments to an instance method that mutate both the instance and the argument. For example, Passenger.book() mutates both the Passenger and Flight instances at the same time. Is this bad design?

In case you are wondering, this is not homework.

```
#!/usr/bin/env python3
import random

letters = 'ABCDEFGHIJKLMNOPQRSTUVWXYZ'

class BookingClass:
ECONOMY = 'Economy'
BUSINESS = 'Business'

class Seat:
def __init__(self, row_number: int, letter: str):
self.row_number = row_number
self.letter = letter

def __str__(self):
return '{}{}'.format(self.row_number, self.letter)

class SeatingArea:
def __init__(self, booking_class: str, start_row: int, row_count: int, seats_per_row: int):
self.booking_class = booking_class
self.seat_count = row_count * seats_per_row
self.seats_remaining = []

for row_number in range(start_row, row_count + start_row):
for seat_number in range(seats_per_row):
new_seat = Seat(row_number, letters[seat_number])
self.seats_remaining.append(new_seat)

class Flight:
def __init__(self, economy_seats: SeatingArea, business_seats: SeatingArea):
# should I use the SeatingArea.booking_class attribute as the key?
self.seating_areas = {
BookingClass.ECONOMY: economy_seats,
BookingClass.BUSINESS: business_seats}

def print_statistics(self):
for booking_class, seating_area in self.seating_areas.items():
print('{}: {}% avail

Solution

Your hardcoded letters is easier to take from string (just in case the alphabet ever changes or to save you some typing in some future code you might write):

from string import ascii_uppercase as letters


I don't see the advantage of

class BookingClass:
    ECONOMY = 'Economy'
    BUSINESS = 'Business'
...
    self.seating_areas = {
            BookingClass.ECONOMY: economy_seats,
            BookingClass.BUSINESS: business_seats}


over

self.seating_areas = {
        "economy": economy_seats,
        "business: business_seats}


The __str__ of Seat could be slightly simplified:

def __str__(self):
    return '{0.row_number}{0.letter}'.format(self)


In the constructor of SeatingArea you don't need the intermediate variable:

for row_number in range(start_row, row_count + start_row):
        for seat_number in range(seats_per_row):
            self.seats_remaining.append(Seat(row_number, letters[seat_number]))


Assigning the booking ID like this:

self.booking_id = random.Random().randrange(1000, 10000)


will mean a roughly a 1:9000 chance of a collision. I don't know how many customers this is supposed to deal with, but that does not seem too unrealistic for e.g. a year of running. Maybe give the Flight a name and date/time like this and use that together with the seat and hash it with md5 to get a more unique id:

from datetime import datetime
from hashlib import md5
...

class Flight:
    def __init__(self, 
                 path: str,
                 date: datetime,
                 economy_seats: SeatingArea,
                 business_seats: SeatingArea):
        ...
        self.path = path
        self.date = date

...

class Passenger:
    ...
    def book(self, flight: Flight, booking_class: str):
        ...
        id_str = "{0.path}{0.date}{1.seat}".format(flight, self)
        self.booking_id = md5(id_str.encode()).hexdigest()

...

    flight_time = datetime.strptime("2016-08-12 8:00:00 -0800" "%Y-%m-%d %H:%M:%S %z")
    flight = Flight("LAX->JFK", flight_time, economy, business)


This way every seat on every flight between two airports for every different starting time has a different id (at least the chance of collisions for md5 are a lot less than 1:9000).

In reality you would probably also have a Plane class, which has the SeatingArea's as parameters and just pass that on to Flight as the plane servicing that particular flight.

Code Snippets

from string import ascii_uppercase as letters
class BookingClass:
    ECONOMY = 'Economy'
    BUSINESS = 'Business'
...
    self.seating_areas = {
            BookingClass.ECONOMY: economy_seats,
            BookingClass.BUSINESS: business_seats}
self.seating_areas = {
        "economy": economy_seats,
        "business: business_seats}
def __str__(self):
    return '{0.row_number}{0.letter}'.format(self)
for row_number in range(start_row, row_count + start_row):
        for seat_number in range(seats_per_row):
            self.seats_remaining.append(Seat(row_number, letters[seat_number]))

Context

StackExchange Code Review Q#138723, answer score: 2

Revisions (0)

No revisions yet.