Skip to content

Turning in my mini project#8

Open
waynekwon wants to merge 3 commits intosd16fall:masterfrom
waynekwon:master
Open

Turning in my mini project#8
waynekwon wants to merge 3 commits intosd16fall:masterfrom
waynekwon:master

Conversation

@waynekwon
Copy link

Frequency count for best performing attacking players from Tottenham Hotspur

Copy link

@runnersaw runnersaw left a comment

Choose a reason for hiding this comment

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

This review is intended to help you with writing readable and understandable code, not to evaluate or improve the functionality of your code.

Your code is generally very readable, with mostly proper and informing names. You commented major sections of code, which was very helpful. The very minor suggestions that I have for you is to improve function naming and documentation.

" \n",
"#define function to count the number of times each word appeared in the tweet\n",
"\n",
"def get_word(x):\n",

Choose a reason for hiding this comment

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

The following suggestions are intended to make the intent of this code clearer to the reader, which is generally very important when working on code. get_word seems to imply that this returns one word. get_word_counts would be a more appropriate name. Naming parameters is also helpful, so a parameter name like word_list would be helpful to a reader. Also, best practice would be to add a comment at the top of each function, like:

def get_word_count(word_list):
    '''
    This function takes a list of words and returns a dictionary where they keys are the words in the list and the
    values are the amount of times that the word appears in the list
    '''
    code here...

"source": [
"#defining function to sort by number of count each player is mentioned\n",
"\n",
"def print_order(d):\n",

Choose a reason for hiding this comment

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

Small suggestions about this function: print_order is a misleading name because this doesn't actually print anything. Please consider changing this name, and adding a comment to this function.

@runnersaw
Copy link

This is some final feedback about your first mini-project.

The report is well-done and answers all of the required questions. The write-up is a little short, but given the scope of the project, that is pretty understandable.

The code runs as expected. It crashes if it doesn’t find any tweets mentioning one of the players in the player list. This is because you are using list comprehension and dictionary lookup without checking whether the key is in the dictionary.

Documentation is great. Every function includes a docstring, and there are comments indicating major sections of code.

Overall, the style of code was good. Much of the code could be moved into functions, which will help readers read and understand portions of code.

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