5
\$\begingroup\$

Is there a shorter way to write this code or parts of it and where can i inculde some try and excepts for validation

   People={"Dan":22,
            "Matt":54,
            "Harry":78,
            "Bob":91}

schoice=input('Enter existing name: ')
while schoice not in People:
    schoice=input('Invalid entry name is non existant, Enter a chosen name: ')
if schoice in People:
    getscore=People[schoice]
    if getscore<50:
        presents=[]
        for x in range(2):
            present=input('Enter present ' +str(x+1))
            while present == '':
                present = input('Invalid. Try again: ')
            presents.append(present)
    if getscore>=100:
        presents=[]
        for x in range (8):
            present=input('Enter present ' +str(x+1))
            while present == '':
                present = input('Invalid. Try again: ')
            presents.append(present)
    if getscore>=50 and getscore <100:
        presents = []
        for x in range (5):
            present=input('Enter present ' +str(x+1))
            while present == '':
                present = input('Invalid. Try again: ')
            presents.append(present)
File=open("presfile.txt",'a')
File.write(schoice)
File.write(str(presents)+'\n')
File.close()
\$\endgroup\$
  • 1
    \$\begingroup\$ Is People meant to be indented so much? As currently this isn't runnable. \$\endgroup\$ – Peilonrayz Jan 10 '17 at 9:01
3
\$\begingroup\$

PEP8

There are quite a number of style-related "errors" in the code. Try to adhere to the Python PEP8 style guide. Here are a few key points for this particular case:

  • Variables and functions are written in snake_case, classes PascalCase and constants (if any) are written fully in UPPERCASE.
  • Use a single space around operators, assignments, comparisons, etc.
  • Use proper indentation (e.g. at People)
  • Use descriptive variable names.

Also use some whitespace here and there to make it more readable.

Some examples from the code:

people instead of People

get_score instead of getscore. Also get_score seems more like a function name rather than a variable name. Perhaps just use score as this is the actual value of the variable.

if get_score >= 100 instead of if getscore>=100

file = open('presfile.txt', 'a') instead of `File=open("presfile.txt",'a')

chosen_name instead of schoice. Perhaps person or chosen_person is even better, as this gets a value from People.


Use functions

Functions make the code more readable and splits up the responsibility of the code in smaller, reusable code. This also makes it a lot easier to test the application with for instance unittest. Once divided into functions, the code can easily be imported into another project, if need be.


You are doubly checking if schoice is in People

while schoice not in People:
    schoice=input('Invalid entry name is non existant, Enter a chosen name: ')
if schoice in People:
    getscore=People[schoice]

Once the if is reached, it is already certain that schoice is in People, otherwise the while loop would not be broken.

The code

presents = []
for x in range (5):
    present=input('Enter present ' +str(x+1))
    while present == '':
        present = input('Invalid. Try again: ')
    presents.append(present)

is repeated 3 times only with a different range. Put it in a separate function. See also: DRY

def get_presents(amount):
    """Return a list of presents equal in size to the given amount"""
    presents = []
    for x in range(amount):
        while True:
            present = input('Enter present #{}: '.format(x+1))
            if present:
                presents.append(present)
                break
            else:
                print('Invalid present, try again.')
    return presents

Now you can call this function with the amount of expected presents and it will return the list of presents: get_presents(5)

The if checks

if score < 50:
    ...
if score >= 100:
    ...
if score >= 50 and score < 100
# can also be written as 50 <= score < 100
    ...

has the code check get_score up to three times. This is not particularly a performance issue in this case, but could also be written as:

if score < 50:
    ...
elif score >= 100:
    ...
else:
    ...

Use with to open the file:

with open('presfile.txt', 'a') as presfile:
    presfile.write(chosen_person)
    presfile.write(str(presents) + '\n')

This ensures that the file is always closed. See also file.close and this answer about with


The code:

def get_presents(amount):
    """Return a list of presents equal in size to the given amount"""
    presents = []
    for x in range(amount):
        while True:
            present = input('Enter present #{}: '.format(x+1))
            if present:
                presents.append(present)
                break
            else:
                print('Invalid present, try again.')
    return presents


def get_presents_for_person():
    """Description of function."""
    people = {
        "Dan": 22,
        "Matt": 54,
        "Harry": 78,
        "Bob": 91,
    }

    chosen_person = input('Enter existing name: ')
    while chosen_person not in people:
        chosen_person = input('The chosen name does not exist. '
                              'Please enter an existing name: ')

    score = people[chosen_person]
    presents = []

    if score < 50:
        presents = get_presents(2)
    elif score >= 100:
        presents = get_presents(8)
    else:
        presents = get_presents(5)

    return chosen_person, presents


if __name__ == '__main__':
    chosen_person, presents = get_presents_for_person()

    with open('presfile.txt', 'a') as presfile:
        presfile.write(chosen_person)
        presfile.write(str(presents) + '\n')

What's next?

You could make the values of People an actual class. This allows you to further expand upon it if necessary in the future. For example something like:

class Person:

    def __init__(self, name, score):
        self.name = name
        self.score = score
\$\endgroup\$

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.