oshimamh / nbaProj

nbaProj
1 stars 0 forks source link

Code review #1

Open TomNash opened 8 years ago

TomNash commented 8 years ago

General Review Checklist

Purpose

Pretty clearly stated, great README writeup

Would consider reorganizing the functions and their file location.

Not a clear flow of how the files interact. Would have files for each set of related functions that can be sourced/loaded, then a separate driver script to use examples.

Good variable naming, pretty clear.

Good commenting.

As above, just consider reorganizing maybe.

Organization

Simple outputs, all data seems to be pulled off web.

Took a couple minutes, README was pretty helpful

Really good spacing and readability.

Change how files are written, would be nice to be able to source every file. Maybe look up how to make an R package if there's just one driver function the user really needs to use. Can load all necessary packages at once, have user install dependencies on load.

Functionality

Really good progress, love the plots

It definitely works well.

As I said, just maybe reorganize the functions within files for easier use.

Specific File Comments

Nothing specific, looks great.

dmcglinn commented 8 years ago

Thanks for the review @TomNash. I agree that with all these R scripts you need a road map for how they interact with one another. The README.md helps in this regard but a master script could make this clearer as @TomNash suggested. Also because there is so many dependencies I would have an R script for the user to help get those installed.