Skip to content

Amarildo Guga - Random Name Generator#30

Open
AmarildoGuga wants to merge 4 commits intoalexnaylor99:mainfrom
AmarildoGuga:main
Open

Amarildo Guga - Random Name Generator#30
AmarildoGuga wants to merge 4 commits intoalexnaylor99:mainfrom
AmarildoGuga:main

Conversation

@AmarildoGuga
Copy link

No description provided.


def Number(name_list_final):
"""
This function will ask the user the number of names they want from the list
Copy link

Choose a reason for hiding this comment

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

Small inconsistency with documentation: "The function..." on line 5 vs "This function..." on line 18. Pick one convention and stick to it

while len(name_list_final) < 2:
name_list = input("Please input more than one name please. ")
name_list_final = list(set(name_list.split()))

Copy link

Choose a reason for hiding this comment

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

Could add a check for invalid names i.e. John7 or John% here

def Names():
"""
The function asks the user for names, checks them, and returns them.
"""
Copy link

@Pova Pova Apr 25, 2023

Choose a reason for hiding this comment

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

Could provide extra information on output format in the function documentation here (also for functions below)

Copy link

@Pova Pova left a comment

Choose a reason for hiding this comment

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

Excellent job fulfilling the project specifications.

Positives:

  • No major bugs or errors with the code
  • Maintains general pythonic code structure
  • Good use of modularity to split up the script

Things to improve:

  • Be more consistent with function documentation and include more detail into input/output data types
  • Consider edge cases you may need to check for (i.e. invalid names)
  • Consider adding commenting throughout your code to explain the logical flow of the script

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