2
\$\begingroup\$

First time posting here, let me apologize ahead of time if this is not the correct format.. Anyway, I am new to programming and I am working on a school assignment to design a vehicle inventory system in Python (assignment details below). The code seems to be working as it should. I am questioning the 'update vehicle' function I defined outside of the class, whether it would make more sense to change to a class method. Or any other feedback anyone has. Thanks. Create a final program that meets the requirements outlined below.

Assignment: Create an automobile class that will be used by a dealership as a vehicle inventory program. The following attributes should be present in your automobile class:

  • private string make
  • private string model
  • private string color
  • private int year
  • private int mileage

Your program should have appropriate methods such as:

  • constructor
  • add a new vehicle
  • remove a vehicle
  • update vehicle attributes

At the end of your program, it should allow the user to output all vehicle inventory to a text file.

class Automobile:

    def __init__(self):
        self.make = " "
        self.model = " "
        self.color = " "
        self.year = 0
        self.mileage = 0


    def add_vehicle(self):
        self.year = int(input("Enter year: "))
        self.make = input("Enter make: ")
        self.model = input("Enter model: ")
        self.color = input("Enter color: ")
        self.mileage = int(input("Enter mileage: "))

    def __str__(self):
        return('%d %s %s Color: %s Mileage: %d' %
              (self.year, self.make, self.model, self. color,
               self.mileage))

vehicle_list = []

def edit(vehicle_list):
    pos = int(input('Enter the position of the vehicle to edit: '))
    new_vehicle = car.add_vehicle()
    new_vehicle = car.__str__()
    vehicle_list[pos-1] = new_vehicle
    print('Vehicle was updated')

user=True
while user:
    print ("""
    1.Add a Vehicle
    2.Delete a Vehicle
    3.View Inventory
    4.Update Inventory
    5.Export Inventory
    6.Quit
    """)
    ans=input("What would you like to do? ") 
    if ans=="1": 
        car = Automobile()
        car.add_vehicle()
        vehicle_list.append(car.__str__())

    elif ans=="2":
        for i in vehicle_list:
            vehicle_list.pop(int(input('Enter position of vehicle to remove: ')))
            print('Successfully removed vehicle')
    elif ans=="3":
        print(vehicle_list)
    elif ans=="4":
        edit(vehicle_list)
    elif ans=='5':
        f = open('vehicle_inv.txt', 'w')
        f.write(str(vehicle_list))
        f.close()
    else:
        print('try again')
\$\endgroup\$
  • \$\begingroup\$ Welcome to Code Review! I hope you get great answers. \$\endgroup\$ – Phrancis Jul 7 '18 at 22:43
3
\$\begingroup\$
  1. The assignment mentions private fields. In Python those are created (faked, really) by prefixing the field with two underscores, as in __make.
  2. Interactive command line programs are a scourge, because they are a pain to automate (including for testing purposes). Have a look at argparse to create a non-interactive program which would be called for example as ./automobile.py add "My car". In general there are no input calls in 99.998% of production code.
  3. I'm not sure whether this recommendation is Pythonic, but I prefer creating a pretty-print method rather than overriding __str__ for printing the object in a user readable format.
  4. The edit method takes a parameter which shadows an existing name. This is a common source of confusion and bugs.
  5. add_vehicle should be inlined into __init__. It doesn't actually add a vehicle (which would mean adding it to something, such as vehicle_list), but rather initializes the vehicle fields, making it effectively a constructor.
  6. Your code does not print the existing fields of a vehicle before replacing it. This may or may not be outside the scope of the assignment, but it is a usability issue.
  7. The user variable serves no purpose and can be inlined.
  8. edit asks for a one-offset index but when deleting a vehicle you use a zero-offset. You can use one or the other (ideally zero-offset for libraries and one-offset for user facing stuff) but it should be consistent.
  9. Use with open(...) to guarantee closing the file cleanly.
\$\endgroup\$
  • 3
    \$\begingroup\$ Regarding 3: Overriding __str__ is pythonic, but calling it directly is not (which is true for all magic methods). One can do str(car) instead. \$\endgroup\$ – Graipher Jul 8 '18 at 5:53
  • \$\begingroup\$ Double underscores are for name mangling, not private attributes. 'Private' attributes, by convention, are prefixed with a single underscore. \$\endgroup\$ – Daniel Jul 8 '18 at 11:10

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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