6
\$\begingroup\$

I'm new to this so I'm not sure if this is allowed but, I recently finished an assigned task and would like to know if there are any ways for me to improve my code.

My task is as follows: In preparation for the examination candidates should attempt the following practical tasks by writing and testing a program or programs.

The organiser of a senior citizens’ club arranges outings for the members. For each of these outings a coach is hired, meals at a restaurant are reserved and tickets for the theatre are booked. A program is required to work out the costs and provide a printed list showing everyone on the outing.

Write and test a program for the club organiser.

• Your program must include appropriate prompts for the entry of data. • Error messages and other output need to be set out clearly. • All variables, constants and other identifiers must have meaningful names.

You will need to complete these three tasks. Each task must be fully tested.

TASK 1 – Work out the total cost of the outing.

The organiser finds out how many senior citizens would be interested in the outing. The program for TASK 1 works out the cost from this information as seen from the table below.

$$ \begin{array}{|l|l|l|l|} \hline \text{Number of people}&\text{Hire of coach (\$)} &\text{Cost of a meal (\$)} &\text{Cost of a theatre ticket (\$)} \\ \hline 12-16 & 150 & 14.00 & 21.00 \\ \hline 17-26 & 190 & 13.50 & 20.00 \\ \hline 27-39 & 225 & 13.00 & 19.00 \\ \hline \end{array} $$

The minimum number of senior citizens needed for the outing to go ahead is 10; there cannot be more than 36 senior citizens on the outing. A minimum of two carers must go on the outing, with an additional carer needed if more than 24 senior citizens go on the outing. Carers do not have to pay anything for the outing. Work out the total cost and the cost per person for the senior citizens.

TASK 2 – Record who is going on the outing and how much has been paid.

Using your results from TASK 1, record the names of the people on the outing and the amount they have paid; include the carers on the outing. If there are spare places on the coach then extra people can be added; they are charged the same price as the other senior citizens. Calculate the total amount of money collected. Print out a list of the people on the outing.

TASK 3 – Identify the break-even point or profit that will be made on the outing.

Show whether the outing has made a profit or has broken even using the estimated cost from TASK 1 and the money collected from TASK 2.

And my code:

seniors = int(input("Please enter the amount of senior citizens going: "))
carers = int(input("Please enter the amount of carers going: "))

if seniors + carers >= int(40) or seniors <= int(1) or seniors >= int(37) or carers <= int(1) or (seniors >= int(24) and carers <= int(2)):
    print("Enter Valid Info Please")

else:
    print("Seniors going: " + repr(seniors) + " | Carers going: " + repr(carers))
    if seniors >= int(1) and seniors <= int(16):

        cost_coach = int(150)
        cost_meal = int(14)
        cost_ticket = int(21)
        seats = int(16)

    if seniors >= int(17) and seniors <= int(26):

        cost_coach = int(190)           
        cost_meal = float(13.50)
        cost_ticket = int(20)
        seats = int(26)

    if seniors >= int(27) and seniors <= int(36):

        cost_coach = int(225)            
        cost_meal = int(13)
        cost_ticket = int(19)
        seats = int(36)

    cost_coach_per_person = float(cost_coach / seniors)
    rounded_cost_coach_per_person = float("%.2f" % cost_coach_per_person)
    total_cost = (cost_meal * seniors) + (cost_ticket * seniors) + cost_coach
    cost_per_person = cost_meal + cost_ticket + rounded_cost_coach_per_person

    print("The total cost is $" + repr(total_cost))
    print("The total cost per person is $" + repr(cost_per_person))

    names_ppl_going = []
    print("Logbook (Names and payment)")
    n = int(1)
    total_payment = float(0)

    for x in range(seniors):
        person_id = repr(n) + ")"
        names_ppl_going.append(person_id)

        name = input("[" + repr(n) +  "] Enter a name: ")
        name2 = name + ":"
        names_ppl_going.append(name2)

        payment = float("%.2f" %(float(input("Amount paid($): "))))
        names_ppl_going.append("$" + repr(payment) + " | ")

        total_payment += payment
        n+=1

    print("Whose going and how much they paid: ")
    print(' '.join(names_ppl_going))
    print("There are " + repr(seats - seniors) + " empty seats left")

    question_3 = int(input("Would you like to fill the extra spaces? If so how many? (Number*/0 to cancel): "))
    if question_3 > 0 and question_3 <= seats - seniors:
        for x in range(question_3):
            person_id = repr(n) + ")"
            names_ppl_going.append(person_id)

            name2 = input("[" + repr(n) +  "] Enter a name: ") + ":"
            names_ppl_going.append(name2)

            names_ppl_going.append("$" + repr(float("%.2f" %(float(input("Amount paid($): "))))) + " | ")

            total_payment += payment
            n+=1
            seniors += 1
        print(' '.join(names_ppl_going))

    else:
        print("Process canceled")

    while True:
        question_1 = input("Do you still need to change anything in the list? (Y/N): ")

        if question_1 == "Y":
            question_2 = input("Would you like to change a name or a payment? (Name/Payment/Exit): ")

            if question_2 == "Name":
                list_change = int(input("Enter the number of the person you want to edit: "))

                if list_change == int(1):              
                    names_ppl_going.pop(1)
                    names_ppl_going = names_ppl_going[:int(list_change)] + [input("Enter the new name: ") + ":"] + names_ppl_going[int(list_change):]
                    print(' '.join(names_ppl_going))

                elif list_change >= int(2):                
                    list_change2 = (3*list_change)-2
                    names_ppl_going.pop(list_change2)
                    names_ppl_going = names_ppl_going[:int(list_change2)] + [input("Enter the new name: ") + ":"] + names_ppl_going[int(list_change2):]
                    print(' '.join(names_ppl_going))

            elif question_2 == "Payment":
                list_change = int(input("Enter the number of the person you want to edit: "))

                if list_change == int(1):
                    convt = ((names_ppl_going[2]).strip('$')).strip(' | ')
                    total_payment -= float("%.2f" %(float(convt)))

                    names_ppl_going.pop(2)
                    payment_change = list_change + int(1)

                    payment_add = float("%.2f" %(float(input("Enter the new payment: "))))
                    total_payment += float(payment_add)

                    names_ppl_going = names_ppl_going[:int(payment_change)] + ["$" + repr(payment_add) + " | "] + names_ppl_going[int(payment_change):]
                    print(' '.join(names_ppl_going))

                elif list_change >= int(2):                
                    list_change2 = (3*list_change)-1
                    convt = ((names_ppl_going[list_change2]).strip('$')).strip(' | ')
                    total_payment -= float("%.2f" %(float(convt)))

                    names_ppl_going.pop(list_change2)                            

                    payment_add_2 = float("%.2f" %(float(input("Enter the new payment: "))))
                    total_payment += float(payment_add_2)

                    names_ppl_going = names_ppl_going[:int(list_change2)] + ["$" + repr(payment_add_2) + " | "] + names_ppl_going[int(list_change2):]
                    print(' '.join(names_ppl_going))                                                        
            else:
                break                        
        else:
            break

    print("Total paid: $" + repr(float("%.2f" %(total_payment))))
    if total_payment < total_cost:
        print("You lost a total of $" + repr(total_cost - total_payment) + "!")
    elif total_payment >= total_cost:
        print("You earnt a profit of $" + repr(total_payment - total_cost))
    print("There are " + repr(seats - seniors) + " empty seats left")

Thanks in advance!

\$\endgroup\$
  • \$\begingroup\$ You forgot about carers. \$\endgroup\$ – vnp Mar 22 '18 at 18:30
  • \$\begingroup\$ In what way did I forget about carers? \$\endgroup\$ – Skryptix Mar 22 '18 at 23:10
  • \$\begingroup\$ I don't see how you account for them. To begin with, carers need seats, so seats - seniors can't be right. \$\endgroup\$ – vnp Mar 23 '18 at 0:34
  • 3
    \$\begingroup\$ An image is neither searchable not its content be easily copied. Would you mind converting it to text? \$\endgroup\$ – 301_Moved_Permanently Mar 23 '18 at 8:15
  • 1
    \$\begingroup\$ 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 you may and may not do after receiving answers. \$\endgroup\$ – Zeta Mar 28 '18 at 12:01
7
\$\begingroup\$
  1. Your first if is complicated. Instead make a function that clearly says what you're testing against.
  2. Keep the grouped prices in a prices object. I'd also keep all the prices in a prices object.
  3. Use a main function.
  4. You can calculate the total cost, and divide by the amount of seniors.
  5. You should group people into an object, and add that object to the names_ppl_going.

    This means that you won't have to times your id's by three, and won't have to have drastically different code in your edit list logic.

  6. Make a person_info function that builds the person for you. This makes your code more DRY.

  7. Don't add formatting to your values until you want to display them. My name isn't Peilonrayz: it's Peilonrayz.
  8. You can remove the need for n if you set proper start and end points on your range.
  9. Calculate the total profit at the end. Calculating it throughout add unneeded complexity. And actually makes the code slower.
  10. str.format is your friend, use it.
  11. I'd really recommend making the names_ppl_going list contain people, and so would add the tuple [id, name, payment] to the list.

    This means that adding to the list is easier, but more importantly editing the list is too.

    I'd then change the editing so you ask if the user wants to edit the list. Then I'd ask which user they want, and select that person in the list. Then you can ask what they'd want to change, and so won't be duplicating the above people check across both. Once you've selected the person, this means there will only be 7 lines of code to update the information.

And so I'd change your code to something like: (Not tested)

from collections import namedtuple


class Prices:
    Price = namedtuple('price', 'amount coach meal theatre')
    SMALL = Price(16, 150, 14, 21)
    MEDIUM = Price(26, 190, 13.5, 20)
    LARGE = Price(39, 225, 13, 19)


def valid_amount_people(seniors, carers):
    # The minimum number of senior citizens needed for the outing to go ahead is 10
    if senior < 10:
        return False

    # there cannot be more than 36 senior citizens on the outing
    if senior > 36:
        return False

    # A minimum of two carers must go on the outing
    if carers < 2:
        return False

    # with an aditional carer needed if more than 24 senior citizens go on the outing.
    if senior > 24 and carers < 3:
        return False

    # We can't have more than 39 people.
    if senior + carers > 39:
        return False

    return True


def person_info(id):
    return [
        id,
        input('[{id}] Enter a name: '),
        float(input('Amount paid($): '))
    ]


def main():
    seniors = int(input('Please enter the amount of senior citizens going: '))
    carers = int(input('Please enter the amount of carers going: '))

    if not valid_amount_people(seniors, carers):
        print('Enter Valid Info Please')
        return

    people = seniors + carers
    prices = next(
        p
        for p in [Prices.SMALL, Prices.MEDIUM, Prices.LARGE]
        if people <= p.amount
    )

    cost = prices.coach + seniors * (prices.meal + prices.theatre)
    print('The total cost is ${}'.format(cost))
    print('The total cost per person is ${}'.format(cost / seniors))

    print('Logbook (Names and payment)')

    end = people + 1
    people_info = [
        person_info(id)
        for id in range(1, end)
    ]

    print('Whose going and how much they paid: ')
    print(' '.join('{}) {}: ${} | '.format(*p) for p in people_info))
    left = prices.amount - people
    print('There are {} empty seats left'.format(left))

    additional_people = input('Would you like to fill the extra spaces? If so how many? ')
    try:
        additional_people = int(additional_people)
    except ValueError:
        additional_people = 0

    if additional_people != 0:
        if additional_people < left:
            print('Not enough seats, adding none.')
        else:
            for id in range(end, end + additional_people):
                people_info.append(person_info(id))

    while input('Do you want to change the list?').lower() == 'y':
        person_id = int(input('Enter the id of the person you want to edit: '))
        index = person_id - 1
        if not (0 <= index < len(person_info)):
            print('Invalid id')
            continue

        person = person_info[index]
        type = input('Would you like to change a name or a payment? (Name/Payment/Exit): ').lower()
        if type == 'name':
            person[1] = input('Enter the new name: ')
        elif type == 'payment':
            person[2] = float(input('Enter the new payment: '))
        else:
            break

    payment = sum(p[2] for p in person_info)
    print("Total paid: ${}".format(payment))
    if payment < cost:
        print("You lost a total of ${}!".format(cost - payment))
    elif payment >= cost:
        print("You earnt a profit of ${}".format(payment - cost))
    print("There are {} empty seats left".format(prices.amount - len(people_info)))
| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ Thanks a lot for the advice! Unfortunately your code doesn't work and I can barely understand half of it XD but the advice is definitely appreciated, would this count as an acceptable answer? \$\endgroup\$ – Skryptix Mar 24 '18 at 15:16
  • \$\begingroup\$ @Skryptix Could you say how my code doesn't work? There's three things 'wrong' with it. I'm feel you can figure out how to fix them :) \$\endgroup\$ – Peilonrayz Mar 24 '18 at 16:20
  • \$\begingroup\$ oh this is gonna take awhile, bare with me as I'm new at programming, well its currently 12:45am where I'm at but i'll make sure to do it first thing in the morning tmr \$\endgroup\$ – Skryptix Mar 24 '18 at 16:46
  • \$\begingroup\$ @Skryptix sure thing, and no problem! I wish you the best :) \$\endgroup\$ – Peilonrayz Mar 24 '18 at 17:53
  • \$\begingroup\$ sorry for the late reply, could u clarify ur intentions behind the line: input('[{id}] Enter a name: ') was the [{id}] intentional or? \$\endgroup\$ – Skryptix Mar 27 '18 at 12:23

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.