License and Registration, Sir

Question

The following code will generate a number plate and then generate a random speed and see if it's speeding

import random, string, time

class Car:
    def __init__(self):
        self.plate = self.genPlate()
        self.limit = 60
        self.checkSpeeding()

    def checkSpeeding(self):
        self.speeding = random.randint(0, 1)

        if self.speeding:
            self.speed = random.randint(self.limit+3, self.limit+20)
        else:
            self.speed = random.randint(self.limit-20, self.limit+2)

    def genPlate(self):
        plateFormat = ['L', 'L', 'N', 'N', 'L', 'L', 'L']
        genPlate = []

        for i in plateFormat:
            if i == 'L':
                genPlate.append(random.choice(string.ascii_letters[26:]))

            elif i == 'N':
                genPlate.append(str(random.randint(0, 9)))

        genPlate.insert(4, " ")

        return  "".join(genPlate)


allCars = []
x = 0

while True:
    allCars.append(Car())

    print(allCars[x].plate + " was going " + str(allCars[x].speed) + "mph in a " + str(allCars[x].limit) + "mph zone")
    if allCars[x].speeding:
        print("speeding fine issued")

    print("\n")

    time.sleep(5)
    x += 1

Here is a link to the original source

Answer 1

I am going to improve the following:

allCars = []
x = 0

while True:
    allCars.append(Car())

    print(allCars[x].plate + " was going " + str(allCars[x].speed) + "mph in a " + str(allCars[x].limit) + "mph zone")
    if allCars[x].speeding:
        print("speeding fine issued")

    print("\n")

    time.sleep(5)
    x += 1

---

Your x variable should be eliminated:

allCars = []

while True:
    allCars.append(Car())

    print(allCars[-1].plate + " was going " + str(allCars[-1].speed) + "mph in a " + str(allCars[-1].limit) + "mph zone")
    if allCars[-1].speeding:
        print("speeding fine issued")

    print("\n")

    time.sleep(5)

[-1] is the last element and append puts the element in last position.

---

Actually you don't even need a list:

while True:
    new_car = Car()

    print(new_car.plate + " was going " + str(new_car.speed) + "mph in a " + str(new_car.limit) + "mph zone")
    if new_car.speeding:
        print("speeding fine issued")

    print("\n")

    time.sleep(5)

---

The print message is very long and not nice, instead use .format():

while True:
    new_car = Car()

    print("{} was going {} mph in a {} mph zone".format(
        new_car.plate,new_car.speed,new_car.limit))
    if new_car.speeding:
        print("speeding fine issued")

    print("\n")

    time.sleep(5)

---

Adding dots to the end of phrases, Capitalizing the first letter, removing unecessary statement and sleeping less:

while True:
    new_car = Car()

    print("{} was going {} mph in a {} mph zone.".format(
        new_car.plate,new_car.speed,new_car.limit))
    if new_car.speeding:
        print("Speeding fine issued.\n")

    time.sleep(1)

---

You should use a main function and if __name__ == "__main__": so that you will be able to import your module without running it:

def main():
    while True:
        new_car = Car()

        print("{} was going {} mph in a {} mph zone.".format(
            new_car.plate,new_car.speed,new_car.limit))
        if new_car.speeding:
            print("Speeding fine issued.\n")

        time.sleep(1)

if __name__ == "__main__":
    main()

You should always strive to make the code the simpler possible.

---

About the rest of the code, minor remarks:

• You should use names_with_underscores and not camelCase because of PEP8
• 60 is hardcoded, it would be nicer to pass in a limit parameter when generating a new car.

Answer 2

Consider the real world scenario a little bit. Does a car know what the speed limit is? Does it issue itself a fine if it's speeding? Or do the police do this? Assuming we're talking about the current state of things and not some near future world where cars do these things, police do.

A more proper OOP approach would be to create a Policeman class with a IsSpeeding method that takes a Car in as an argument. The Policeman would then be responsible for determining whether or not the car was speeding and issuing tickets when necessary.

Answer 3

I am going to get pedantic about formatting (see PEP-0008). I have left comments about the formatting in the code below.

# Each import gets its own line.
import random
import string
import time

# Always inherit classes from object when you are not inheriting
# from another class.
class Car(object):
    def __init__(self):
        # Naming: camelCase is generally frowned upon, classes
        # get PascalCase and vars and functions have_underscores.
        self.plate = self.gen_plate()
        self.limit = 60
        self.check_speeding()

    def check_speeding(self):
        self.speeding = random.randint(0, 1)

        if self.speeding:
            # Operators always have spaces before and after them.
            self.speed = random.randint(self.limit + 3, self.limit + 20)
        else:
            self.speed = random.randint(self.limit - 20, self.limit + 2)

    # Does not really need any access to Car's members.
    @staticmethod
    def gen_plate():
        # You can iterate over a string just fine.
        plate_format = "LLNN LLL"
        # Avoid confusion between Car.gen_plate and the list
        plate = []

        for i in plate_format:
            if i == 'L':
                plate.append(random.choice(string.ascii_letters[26:]))
            elif i == 'N':
                plate.append(str(random.randint(0, 9)))
            else:
                # No need for inserts, it is better to do everything
                # in one place.
                plate.append(i)

        return "".join(plate)

# So that it does not start executing if someone loads your code
# as a library.
if __name__ == "__main__":
    # Removed x and all_cars because you never access any
    # data from previously created cars.
    while True:
        car = Car()    
        # String concatenation is not very readable.
        print("%s was going %i mph in a %i mph zone" %
              (car.plate, car.speed, car.limit))
        if car.speeding:
            print("speeding fine issued")
        print("\n")
        time.sleep(5)