Skip to content

Turning in Eric's mini project 1#13

Open
lqyeric94 wants to merge 4 commits intosd16fall:masterfrom
lqyeric94:master
Open

Turning in Eric's mini project 1#13
lqyeric94 wants to merge 4 commits intosd16fall:masterfrom
lqyeric94:master

Conversation

@lqyeric94
Copy link

No description provided.

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.

In addition to the comments I made below, improving the names of your variables to allow a user to see what they are would be good.

"from pattern.web import *\n",
"import string\n",
"\n",
"christianity=URL(\"http://www.gutenberg.org/cache/epub/8294/pg8294.txt\").download()\n",

Choose a reason for hiding this comment

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

You can move the declaration of the variable to the line before you process them, such as:

christianity=URL("http://www.gutenberg.org/cache/epub/8294/pg8294.txt").download()
process_file(christianity)

This improves the readability of your code by grouping related things.

" if wordss in dic:\n",
" dic[wordss] += 1\n",
" else:\n",
" dic [wordss] = 1\n",

Choose a reason for hiding this comment

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

Best practice is to not put a space before the square brackets here.

"\n",
"\n",
" \n",
"def process_file(filename):\n",

Choose a reason for hiding this comment

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

It would be good to add a comment describing what this function does at the top of the function, such as:

def process_file(file):
    '''
    This function does something blah blah
    '''
    code here...

@runnersaw
Copy link

This is some final feedback about your project.

Overall, your report answered every question that was asked of you. Your answers were somewhat short, but given the brevity of the code and the scope of the project, I’m not sure that there was much more for you to say.

The code runs without modification. However, there is a mistake where you do the following:

   for a,b in le:
        return b

This means that it only returns the top result, rather than printing or returning the top number of results. Changing this to a print, or printing le solves this issue rather simply.

There isn’t a single comment or docstring in your code.

Your code is minimal and organized well. Variable names could use improvement. You redefine a function unnecessarily (you can define the function once, and then use it multiple times).

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