Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I am learning about object oriented programming in Python using classes. I have attached an image of what I had in mind when structuring my classes. This is supposed script model cars. I want to know if the way that I set up my code it the ideal way of modeling cars.

A few things:

  • There are no syntax errors. I can create instances without errors.

  • The subclasses uses *args to get the attributes of the parent class. Is this the correct way of doing that?

  • I am using Python 2.7.5

  • I have included a few example instances of the class

  • Each class only has an __init__ method and a vehicle_print method. I have not yet added any other methods.

enter image description here

class Vehicle(object):  # no instance of this class should be created

    def __init__(self, typ, make, model, color, year, miles):
        self.typ = typ
        self.make = make
        self.model = model
        self.color = color.lower()
        self.year = year
        self.miles = miles

    def vehicle_print(self):
            print('Vehicle Type: ' + str(self.typ))
            print('Make: ' + str(self.make))
            print('Model: ' + str(self.model))
            print('Color: ' + str(self.color))
            print('Year: ' + str(self.year))
            print('Miles driven: ' + str(self.miles))


class GasVehicle(Vehicle):

    def __init__(self, fuel_tank, *args):
        self.fuel_tank = fuel_tank
        Vehicle.__init__(self, *args)

    def vehicle_print(self):
        Vehicle.vehicle_print(self)
        print('Fuel capacity (gallons): ' + str(self.fuel_tank))


class ElectricVehicle(Vehicle):

    def __init__(self, energy_storage, *args):
        self.energy_storage = energy_storage
        Vehicle.__init__(self, *args)

    def vehicle_print(self):
        Vehicle.vehicle_print(self)
        print('Energy Storage (Kwh): ' + str(self.energy_storage))


class HeavyVehicle(GasVehicle):  # no instance of this class should be created

    def __init__(self, max_weight, wheels, length, *args):
        self.max_weight = max_weight
        self.wheels = wheels
        self.length = length
        GasVehicle.__init__(self, *args)

    def vehicle_print(self):
        GasVehicle.vehicle_print(self)
        print('Maximum load (tons): ' + str(self.max_weight))
        print('Wheels: ' + str(self.wheels))
        print('Length (m): ' + str(self.length))


class ConstructionTruck(HeavyVehicle):

    def __init__(self, cargo, *args):
        self.cargo = cargo
        HeavyVehicle.__init__(self, *args)

    def vehicle_print(self):
        HeavyVehicle.vehicle_print(self)
        print('Cargo: ' + str(self.cargo))


class Bus(HeavyVehicle):

    def __init__(self, seats, * args):
        self.seats = seats
        HeavyVehicle.__init__(self, *args)

    def vehicle_print(self):
        HeavyVehicle.vehicle_print(self)
        print('Number of seats: ' + str(self.seats))


class HighPerformance(GasVehicle):  # no instance of this class should be created

    def __init__(self, hp, top_speed, *args):
        self.hp = hp
        self.top_speed = top_speed
        GasVehicle.__init__(self, *args)

    def vehicle_print(self):
        GasVehicle.vehicle_print(self)
        print('Horse power: ' + str(self.hp))
        print('Top speed: ' + str(self.top_speed))


class SportCar(HighPerformance):

    def __init__(self, gear_box, drive_system, *args):
        self.gearbox = gear_box
        self.drive_system = drive_system
        HighPerformance.__init__(self, *args)

    def vehicle_print(self):
        HighPerformance.vehicle_print(self)
        print('Gear box: ' + self.gearbox)
        print('Drive system: ' + self.drive_system)




bmw = GasVehicle(30, 'SUV', 'BMW', 'X5', 'silver', 2003, 120300)  # regular car
bmw.vehicle_print()
print
tesla = ElectricVehicle(85, 'Sport', 'Tesla', 'Model S', 'red', 2014, 1243)  # electric car
tesla.vehicle_print()
print
lambo = SportCar('manual', 'rear wheel', 650, 160, 23, 'race car', 'Lamborgini', 'Enzo', 'dark silver', 2014, 3500)  # sportscar
lambo.vehicle_print()
print
truck = ConstructionTruck('cement', 4, 12, 21, 190, 'transport', 'Dirt Inc.', 'Dirt Blaster 100', 'blue', 1992, 120030)  # Construction truck
truck.vehicle_print()
share|improve this question
    
Your class hierarchy makes no sense. There are electric buses, trucks, and sports cars. –  Gareth Rees Jan 28 at 13:25

3 Answers 3

up vote 7 down vote accepted

A couple of things to consider

1) As a style thing, I'd prefer **kwargs to *args in your constructors. This makes things clearer since there's no chance that a change to a base class initializer will invalidate other code in subclasses. Something like @MichaelUrman's example:

class Vehicle(object):
    """Represent a vehicle."""
    def __init__(self, **kwargs):
        self.info = kwargs

Allows you to flexibly specify values while providing class level defaults that vary on application. Super (as mentioned in other answers) is great for avoiding repetition here. For example

class FourByFour (Vehicle):
    def __init__(self, **kwargs):
        # provide a default transmissions
        if not 'transmission' in kwargs: kwarg['transmission'] = 'Generic 4x4'
        super (FourByFour, self).__init__(**kwargs)

In the comments to @MichaelUrman's example, OP asks how to create a specific class of car. A common python idiom (which is less common in other languages) is to create 'thin' subclasses that vary primarily by default content and then occasionally override default methods. This is common in cases where you want a lot of classes which are mostly the same but differ in details and you don't expect to manage lots of subtle hierarchical changes. From the 10,000 ft level this is the same using helper classes, since you're just populating a set of variables in a structured way; I prefer it to using helper functions because it allows you to add custom methods as necessary while keeping things data driven.

For example:

class Vehicle(object):   
"""Represent a vehicle."""
    DEFAULTS = {'wheels':4, 'doors':2, 'fuel':'gasoline', 'passengers':2}

    def __init__(self, **kwargs):
       kwargs.update(self.DEFAULTS)
       self.info = kwargs 

    def drive(self):
       print "Vroom"


class Sedan(Vehicle):
    DEFAULTS = {'wheels':4, 'doors':4, 'fuel':'gasoline', 'passengers':5}

def Hybrid(Sedan):
    DEFAULTS = {'wheels':4, 'doors':4, 'fuel':'smug', 'passengers':4}

    def drive(self):
        print "purrrrrr..."

class Van(Vehicle):
    DEFAULTS = {'wheels':4, 'doors':3, 'fuel':'gasoline', 'passengers':2, 'cargo':[]}

    def unload(self):
       if not self.cargo:
          print "no cargo loaded"
       else:
          print "unloading"
       return self.cargo

Conceptually this is halfway between 'traditional' inheritance heavy OOP and @MichaelUrman's data customization approach. All three have their applications, and the neat thing about python is that it's very good at both @MichaelUrman's method and mine -- which is not true for languages obsessed with type-checking and signature maintenance.

2) On the strategy level, in the long term you'll benefit from a finer grained approach. Engines, transmissions, and tires for example might be their own classes instead of aspects of vehicle (a station wagon and a sedan, for example, might share everything but body styles). The class hierarchy becomes less of an issue as you do more mix-and-match customization by creating collections of components. The usual pretentious programmer way to say this is 'prefer composition over inheritance'

share|improve this answer

When chaining constructors, it's customary to call the parent class constructor first, then perform the subclass-specific initialization. Here, it makes little difference since you're just setting a bunch of variables, but in more complex situations you could run into problems doing things in this order.

Use the super keyword to avoid hard-coding the class hierarchy more than necessary.

Instead of defining vehicle_print(), it would be more customary to define __str__(). It would also give the caller the flexibility to do something other than printing to sys.stdout.

share|improve this answer
    
I disagree with the suggestion to use super here, at least for __init__. While super is designed to solve the diamond inheritance problem, it requires the multiple inheritance call chain's function parameters to remain compatible. Vader's __init__ parameters are incompatible. But it would work fine in vehicle_print / __str__. –  Michael Urman Jan 27 at 20:20

This is a fine place to start learning about OOP class hierarchies. It helps you understand how methods and data are shared amongst various members of the hierarchy. However I would never model a data-hierarchy in this fashion. In fact, especially in python, I would probably make just one class with a bunch of optional attributes. The one implementation of vehicle_print would conditionally print each of the attributes, depending on whether they had a real value. But first let's look more at what you built.

Your use of calling the base class's implementation of vehicle_print is spot on for the hierarchy you've built. This lets you effectively share the implementation of your print function for the data that is shared. The interface of the function, however, is a little limiting, as you do not expose other functionality of the print statement. If you wanted to expose this, there are two main approaches:

  • Add parameters to your function for other capabilities of the print statement, such as a file=sys.stdout parameter like is done in Python 3's print function. You could then use this in your print statement.
  • Instead of implementing a printing function, implement a string-making function such as __str__. This allows the consumer to make those decisions explicitly. If you want to offer a vehicle_print function as well, it can become effectively print str(self).

Your use of *args in __init__ feels a little unusual to me. This is because I'm used to the combined arguments growing in the other order. Instead of having the arguments to SportsCar be (<SportsCar>, <HighPerformance>, <GasVehicle>, <Vehicle>), I typically expect (<Vehicle>, <GasVehicle>, <HighPerformance>, <SportsCar>). This way when looking at initializations of different types, the first arguments are the same across types; your way it's the last arguments. Unfortunately for that order to work, you end up having to name all arguments:

class GasVehicle(Vehicle):
    def __init__(self, typ, make, model, color, year, miles, fuel_tank):
        self.fuel_tank = fuel_tank
        Vehicle.__init__(self, typ, make, model, color, year, miles)

or go with something unhelpful in your function signature:

class GasVehicle(Vehicle):
    def __init__(self, *args):
        if len(args) != 7:            # optionally verify length...
            raise ValueError("...")
        args = list(args)
        self.fuel_tank = args.pop()
        Vehicle.__init__(self, *args) # ...or let the base catch it

Finally, here's the approach I would actually take for the problem at hand. The downside is it doesn't actually leverage OOP, so if you're in a class studying it, you would likely be graded poorly for going this way. (By the way, in Python 3 I might make all the helper function's arguments keyword only, so they do not accept positional parameters, as positional become nearly unreadable in the lengths required below.)

class Vehicle(object):
    """Represent a vehicle."""
    def __init__(self, **kwargs):
        self.info = kwargs

    def __str__(self):
        strs = []
        # base Vehicle info is always known
        strs.append('Vehicle Type: ' + str(self.info['typ']))
        strs.append('Make: ' + str(self.info['make']))
        # ...
        # specialized info is not always known
        if 'fuel_tank' in self.info:
            strs.append('Fuel capacity (gallons): ' + str(self.info['fuel_tank'])
        if 'energy_storage' in self.info:
            strs.append('Energy Storage (Kwh): ' + str(self.info['energy_storage'])
        # ...
        return "\n".join(strs)

# helper functions to create concrete kinds of cars
def ElectricVehicle(typ, make, model, color, year, miles, energy_storage):
    # trick; equivalent to passing each element of the locals() dict by keyword
    return Vehicle(**locals())
def GasVehicle(typ, make, model, color, year, miles, fuel_tank):
    return Vehicle(**locals())
def SportsCar(typ, make, model, color, year, miles, fuel_tank, hp, top_speed, gear_box, drive_system):
    return Vehicle(**locals())
def ConstructionTruck(typ, make, model, color, year, miles, fuel_tank, max_weight, wheels, length, cargo):
    return Vehicle(**locals())
def Bus(typ, make, model, color, year, miles, fuel_tank, max_weight, wheels, length, seats):
    return Vehicle(**locals())
share|improve this answer
    
Your way of doing it in one class look intriguing but I wonder how do i create an instance of lets say sports car or bus? There is only one class for all cars. –  Vader Jan 27 at 20:34
    
@Vader Right, that question doesn't make sense for this implementation. My general assertion here is it's not of practical use to differentiate by type of vehicle. So first we'd have to figure out why you want to differentiate, and then we can consider a means to do so. (Perhaps something like checking 'gear_box' in vehicle.info.) –  Michael Urman Jan 27 at 20:51

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.