Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pull Request for SoftDes17 #13

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

victorbianchi
Copy link

No description provided.

@mightydeveloper
Copy link

mightydeveloper commented Oct 16, 2017

Regarding Documentation,

  • I don't see any README file... (Did you forget to git add before you git commit?)
  • There is no doctring or comments besides the header comment.

Regarding Style,

  • There is no function
  • I don't see any modularity within the code

Regarding Functionality,

  • There is a glitch where if I arrive to the article 'Psychology', the program crashes
  • The experiment is quite interesting, but I'm not convinced how this is related to text mining. (You might want to add some documentation to convince me in the readme file.)
  • It would been a lot better if you made a bunch of experiments on different starting article and show us statistics on the experiment. (Some would reach in x amount of links and some would loop forever)

I feel sorry for being negative, but I can see a lot of room for improvement just by following the class materials or the project webpage. Overall, I think you can make code more readable by making functions and adding docstrings for those functions. In addition, you can make some more experiments on different starting nodes and show us interesting statistics.

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.

None yet

2 participants