Skip to content

Instantly share code, notes, and snippets.

@nickhartdev
Created September 3, 2020 17:08
Show Gist options
  • Save nickhartdev/44a6e2fb659768f2d0b5784902cf965d to your computer and use it in GitHub Desktop.
Save nickhartdev/44a6e2fb659768f2d0b5784902cf965d to your computer and use it in GitHub Desktop.

Code Review

Overall

I really like how you integrated the conditional inside of the moviesList variable, that's really smart. Code organization overall is very clean, and the implementation is pretty dry. This is super solid.

Biggest thing that could be improved on is the testing coverage - there's some unit/integration tests missing for some of the newer iterations, especially the solo one.

At a glance, I do see one opportunity to dry things up - the conditional render inside of the card component seems a little repetitive. You could maybe consider writing a helper function above the return statement checking the value of isFavorite and returning the respective icon.

Another small thing for dev empathy (and this is super nitpicky) would be to logically group your functions if they're in the same file - I had to scroll around a lot to get an understanding of how you implemented the feature. Not a super big deal, but could be something to think about if y'all have the time at the end of a project.

Rubric

Specification Adherence

3 - You covered everything the rubric asked for, functionality is solid. One small fix - the favoriting button on the movie details page doesn't work if the screen is scrunched, as the detail text covers it up. Otherwise this is great.

Project Professionalism

REALLY Solid 3 - I'm taking notes on how solid y'all's workflow was. The only reason this isn't a 4 is because of the linter, but again, I think that's something a lot of us overlooked.

React Architecture

3 - Not much to say here, this is great. Props to y'all for really keeping a lot of the conditional logic out of the returns/render methods. There's a little bit of drying up to be done (specifically in the Card component, as mentioned above) but all in all awesome stuff.

Testing

2 - The tests you have are solid, but there could be more coverage, especially around the solo iteration.

Routing

3 - The router functionality is good, everything works as expected. I also like the added UX benefit of the favorite button popping up when you hover over it - it made it clearer that I could use it (specifically in the movie details page). One small critique - this is just a personal preference, I'd ask the instructors more about conventions for this, but it took me a bit to realize the logo in the top left was the link to home page. I was using the back button a lot.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment