flixster codereview 3 (file re-org and utils)#55
Open
amotor95 wants to merge 32 commits intocodepath:mainfrom
Open
flixster codereview 3 (file re-org and utils)#55amotor95 wants to merge 32 commits intocodepath:mainfrom
amotor95 wants to merge 32 commits intocodepath:mainfrom
Conversation
- Rename fetchMovieList() to fetchAndProcessMoviesByIDList() for clarity - Split fetchAndProcessMoviesByIDList() into fetchMovieByID() and processMoviesByID() - Rename fetchSearchMovies() to fetchAndProcessMoviesBySearch() for clarity - Split fetchAndProcessMoviesBySearch into buildMovieSearchURL(), fetchMoviesBySearch(), and processMoviesBySearch() - Refactor sortMovies() to sortMovieOrder() for clarity and replace if/else with switch/case for readability - Update useEffect dependencies [mode, pageCleared] to use switch/case for readability These changes improve code readability and maintainability by clarifying function purposes and enhancing control flow readability.
…ishing Refactoring Movie Fetch Functions and Sorting
… favorites/watched and then back with text in searchText
…}, moved modal component to movielist component
fix - removed testing console logs fix - renamed promises to moviePromises for readability
…ishing Jackmccl/flixster/styling and polishing
…ishing Fixed minimum movie card width to 90px
…or jsx and css Fix - sorting mode now resets when swapping to favorites or watched
Jackmccl/file reorg Fixing sidebar sort mode bug moved api and sorting to utils
anykac
reviewed
Jun 16, 2025
Comment on lines
+4
to
+11
| :root { | ||
| --lucida-font: 'Lucida Sans', 'Lucida Sans Regular', 'Lucida Grande', 'Lucida Sans Unicode', Geneva, Verdana, sans-serif; | ||
| --steelgray: #1e1e2f; | ||
| --martinique: #2a2a4c; | ||
| --butterflybush: #505091; | ||
| --wildblueyonder: #8080bc; | ||
| --gallery: #f0f0f0; | ||
| } |
| @@ -0,0 +1,66 @@ | |||
| export const fetchMovieByID = async (movieID) => { | |||
There was a problem hiding this comment.
nice work organizing helper functions into a util to reduce code clutter :)
| // Takes a list of movie entries (a list of {id: int, movie: {}}) and sorts by vote average property (highest to lowest) | ||
| export const movieEntriesVoteAverageSort = (movieEntries) => { | ||
| movieEntries.sort((left_entry, right_entry) => { | ||
| return right_entry[1].vote_average-left_entry[1].vote_average |
|
LGTM |
auroraw9825
reviewed
Jun 18, 2025
auroraw9825
left a comment
auroraw9825
left a comment
There was a problem hiding this comment.
Awesome work!
Definitely see very big improvement on code quality, thanks for following up on our comments and you learn so fast!
| const data = await response.json() | ||
| return data | ||
| } catch (error) { | ||
| console.error(error) |
There was a problem hiding this comment.
qq" when there is an error fetching movies, is it a blank page?
| let response = null | ||
| response = await fetch(`https://api.themoviedb.org/3/movie/${movieID}/videos?language=en-US`, options) | ||
| if (!response.ok) { | ||
| throw new Error('Failed to fetch videos') |
There was a problem hiding this comment.
same question here: wondering what the behavior is when fetching is failed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Reorganized components to each have their own folder with their jsx and css
Refactored api calls into apiUtils in utils folder
Refactored sorting methods into sortingUtils in utils folder
Testing
Tested search, sorting, favorites, watched, and movie modal components to ensure functionality after refactoring (components that use API calls and sorting in utils) ✅
