{"action":"created","issue":{"url":"https://api.github.com/repos/dwyl/english-words/issues/46","repository_url":"https://api.github.com/repos/dwyl/english-words","labels_url":"https://api.github.com/repos/dwyl/english-words/issues/46/labels{/name}","comments_url":"https://api.github.com/repos/dwyl/english-words/issues/46/comments","events_url":"https://api.github.com/repos/dwyl/english-words/issues/46/events","html_url":"https://github.com/dwyl/english-words/pull/46","id":318744649,"number":46,"title":"Refactored the python code example","user":{"login":"Stack-of-Pancakes","id":6391930,"avatar_url":"https://avatars3.githubusercontent.com/u/6391930?v=4","gravatar_id":"","url":"https://api.github.com/users/Stack-of-Pancakes","html_url":"https://github.com/Stack-of-Pancakes","followers_url":"https://api.github.com/users/Stack-of-Pancakes/followers","following_url":"https://api.github.com/users/Stack-of-Pancakes/following{/other_user}","gists_url":"https://api.github.com/users/Stack-of-Pancakes/gists{/gist_id}","starred_url":"https://api.github.com/users/Stack-of-Pancakes/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Stack-of-Pancakes/subscriptions","organizations_url":"https://api.github.com/users/Stack-of-Pancakes/orgs","repos_url":"https://api.github.com/users/Stack-of-Pancakes/repos","events_url":"https://api.github.com/users/Stack-of-Pancakes/events{/privacy}","received_events_url":"https://api.github.com/users/Stack-of-Pancakes/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":0,"created_at":"2018-04-29T19:56:26Z","updated_at":"2018-05-19T14:44:11Z","closed_at":null,"author_association":"NONE","pull_request":{"url":"https://api.github.com/repos/dwyl/english-words/pulls/46","html_url":"https://github.com/dwyl/english-words/pull/46","diff_url":"https://github.com/dwyl/english-words/pull/46.diff","patch_url":"https://github.com/dwyl/english-words/pull/46.patch"},"body":"Problems as listed in the commit message\r\n\r\n- `sys` imported and unused.\r\n- `os` only used to create cwd which is implied with `open`\r\n- `try`/`except` block that doesn't handle anything\r\n- `json` loaded to create a more expensive defaultdict\r\n- dict created just for membership testing instead of set\r\n- membership test performed by fetching key value instead of using in\r\n\r\nWhen I first cloned the repo I used the example and I went \"Great, this works\".  Then I looked at it and was really confused with how it was working.\r\n\r\nThe `open` command by default tries to open the file in the `.` directory (current working).  So creating an absolute path was unneeded.\r\n\r\nThe `json.load()` reads a `.json` file and returns a `dict`.  It was the equivalent of iterating over the plain text file and creating a `dict` entry with `valid_words[word] = 1`.\r\n\r\nThe `try`/`except` block does catch some errors but it just returns it as a string.  So when you attempt to lookup the work \"fate\" it would have thrown `TypeError: string indices must be integers` which would have made troubleshooting much more difficult.  If the exception isn't going to be handled, it's best to just let it raise itself and the actual problem will occur immediately, like a `FileNotFound` exception which is much easier to diagnose.\r\n\r\nLastly, when membership testing in Python it's almost always best to use the `in` keyword.  Just trying to reference the key directly results in exceptions that need to be handled and makes the code less readable.  For example if instead of \"fate\" you looked up \"fatex\" it would throw a `KeyError` exception.  So you can either change the `dict` lookup to `english_words.get('fatex', 0)` or even better you can use the `in` keyword on dicts `'fatex' in english_words`.  Though at that point why have a dict if the values never matter, which is why it was changed to return a `set()`."},"comment":{"url":"https://api.github.com/repos/dwyl/english-words/issues/comments/390409717","html_url":"https://github.com/dwyl/english-words/pull/46#issuecomment-390409717","issue_url":"https://api.github.com/repos/dwyl/english-words/issues/46","id":390409717,"user":{"login":"ironhaven","id":23583359,"avatar_url":"https://avatars2.githubusercontent.com/u/23583359?v=4","gravatar_id":"","url":"https://api.github.com/users/ironhaven","html_url":"https://github.com/ironhaven","followers_url":"https://api.github.com/users/ironhaven/followers","following_url":"https://api.github.com/users/ironhaven/following{/other_user}","gists_url":"https://api.github.com/users/ironhaven/gists{/gist_id}","starred_url":"https://api.github.com/users/ironhaven/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ironhaven/subscriptions","organizations_url":"https://api.github.com/users/ironhaven/orgs","repos_url":"https://api.github.com/users/ironhaven/repos","events_url":"https://api.github.com/users/ironhaven/events{/privacy}","received_events_url":"https://api.github.com/users/ironhaven/received_events","type":"User","site_admin":false},"created_at":"2018-05-19T14:44:11Z","updated_at":"2018-05-19T14:44:11Z","author_association":"NONE","body":"The new code looks really good. I learned some new stuff from looking at your new script. This should be merged. "}}
{"actor":{"display_login":"ironhaven"}}