4
\$\begingroup\$

Recently, our High School teacher gave us a pseudocode assignment for the preparation of CAIE's which was a Railway Management System. Everyone completed the task in pseudocode which for me was quite an inflexible program. So I tried my own code which is very much flexible than others. It took me a day to complete the code but there were some bugs and errors which took me almost another day to fix. Hence, can anyone comment on my code and my programming skills? And guide me with some improvements where necessary?

departure = ['09:00', '11:00', '13:00', '15:00'] # Timings of Train Departures
arrival = ['10:00', '12:00', '14:00', '16:00'] # Timings of Train Arrivals
depart_trains = 4
arrival_trains = 4
coaches = 6
total_seats = 80
total_passengers = [0]*len(departure)
ticket_price = 25

while departure:
    print(f"""{depart_trains} trains departing at: {departure}
{arrival_trains} trains arriving at: {arrival}
Price of Ticket/person: {ticket_price} dollars
Each Train has {coaches} coaches, each coach holding {total_seats} passengers :)
""")
    commute = input("Enter Departure time: ").strip()
    if commute in departure:
        index_train = departure.index(commute)
        print(f"""You chose Train departing at: {departure[index_train]}
""")
        run = True
        while run:
            current_passengers = int(input("How many passengers are to sit: "))
            if current_passengers <= coaches*total_seats:
                total_passengers[index_train] += current_passengers
                if total_passengers[index_train] == coaches*total_seats:
                    departure.remove(departure[index_train])
                    arrival.remove(arrival[index_train])
                    total_passengers.pop(index_train)
                    depart_trains -= 1
                    arrival_trains -= 1
                    print(f"""Your total price is: {current_passengers*ticket_price} dollars 
""")
                    run = False
                elif total_passengers[index_train] < coaches*total_seats:
                    print(f"""Your total price is: {current_passengers*ticket_price} dollars
""")
                    run = False
                elif total_passengers[index_train] > coaches*total_seats:
                    total_passengers[index_train] -= current_passengers
                    print(f"""The Train departing at {departure[index_train]} has only {(coaches*total_seats)-total_passengers[index_train]} seats left :(
""")
            else:
                print(f'''Only {coaches*total_seats} seats are available in Train departing at {departure[index_train]} :( 
''')
else:
    print("All Trains are reserved. Come Tomorrow :( ")
\$\endgroup\$
4
  • \$\begingroup\$ Welcome to CodeReview. Does your code work as expected? \$\endgroup\$ Commented Feb 8, 2021 at 11:02
  • 2
    \$\begingroup\$ Welcome to Code Review! Please do not update the code in your question to incorporate feedback from answers, doing so goes against the Question + Answer style of Code Review. This is not a forum where you should keep the most updated version in your question. Please see What should I do when someone answers my question? as well as what you may and may not do after receiving answers. \$\endgroup\$ Commented Feb 8, 2021 at 17:14
  • 4
    \$\begingroup\$ Note- in that version 4 you stated: "But I am getting an IndexError for something which I am not able to find yet. Can anyone suggest me the bug that is causing this?" that would me the post off-topic so keep that in mind for future questions. \$\endgroup\$ Commented Feb 8, 2021 at 17:16
  • 2
    \$\begingroup\$ If you honestly believed your code was working, but discovered a bug after posting it, then it would have been appropriate to write an answer to your own question. A really good answer would show how you discovered the bug, and suggest a fix. \$\endgroup\$ Commented Feb 9, 2021 at 7:59

1 Answer 1

3
\$\begingroup\$

There's nothing that looks bad at a first glance. Couple observations though:

  • It isn't clear what happens if the time entered isn't in the departure list. Consider adding a default case, even if just to explain to the user that he has to punch in a time present in the list.

  • A negative number of passengers is valid according to your program, but I doubt this is intentional. Always validate user input.

  • An exception is thrown if I enter a string instead of a number when asked for the passenger count. See above ;)

  • At first read, I thought total_seats was, well, the total number of seats but it is the number of seats per coach according to the script output. You might want to clarify that or rename the variable.

  • You can use \n in your prints to add newlines, I'm not a fan of binding the formatting of your code to the formatting of your output. Both of these lines print the same thing:

print(f"""Your total price is: {current_passengers*ticket_price} dollars 
""")
print(f"""Your total price is: {current_passengers*ticket_price} dollars \n""")
  • The run variable isn't as self-documenting as it could be if it was named incorrectInput or retryInput or something like that.

Take these two more as suggestions than actual issues with your code:

  • The fact that your timing system relies on list indexes isn't very pythonic (Explicit is better than implicit). You could use a dict to assign a unique ID to every train... But you'd need a bit more code to reproduce index_train = departure.index(commute), and I'm not a fan of doing syntax gymnastics in "exam code" so I'd say that your solution is good enough.
  • Another pythonic thing, "Ask forgiveness not permission": you could get rid of the check for wether commut is in departure, do index_train = departure.index(commute) right away and handle the ValueError that gets thrown if commute isn't in departures using a try except block. Your solution is already fine though (and probably less verbose).

Overall I don't see anything that doesn't work as intended, so it's already pretty solid (as long as the user isn't messing with the input like I did). If it is "exam code" I'd prioritize the formatting and documentation stuff so that your code is more readable. Readable = fast to grade, and fast to grade = less time spend analyzing and figuring out every minor issue with the code ;)

\$\endgroup\$
1
  • \$\begingroup\$ Thanks a lot, I have revamped my question thou, can you check it for me please? \$\endgroup\$ Commented Feb 8, 2021 at 17:10

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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