Skip to content

Alexander-Tkachenko, commiting final code for mini python project#27

Open
Alex-Polishchuk wants to merge 1 commit intoalexnaylor99:mainfrom
Alex-Polishchuk:main
Open

Alexander-Tkachenko, commiting final code for mini python project#27
Alex-Polishchuk wants to merge 1 commit intoalexnaylor99:mainfrom
Alex-Polishchuk:main

Conversation

@Alex-Polishchuk
Copy link

final python project submission

Randomly select x amount of names and output the list
"""

#Asks user for their shortlist of names
Copy link

@ansonchf ansonchf Apr 25, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be better to move these executable lines of code after the functions? So the reviewer can understand how all the functions work before executable lines. It may help the reviewer to understand the logic better.

while True:
try:
#Asks user to input number from 1 to original list length - 1
print('Input the length of list you would like returned')

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe consider to covert these two lines of code to one line.

Suggestion:
print(f'Input the length of list you would like returned\nMin: 1 | Max: {user_length-1}')

print("Please enter a valid number")

#Variable set to length input from user
user_output_length = length_input(user_length)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Executable line in the middle of the script could sometimes make the reviewer to read the logic. Maybe it would be a great idea to move all the executable lines to the end of the script

user_list.pop(temp)

#For loop to print all the names in the list
print('The names chosen are:')

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it is a good option to use the code suggested below.

print('The names chosen are:', *user_list, sep='\n- ')

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants