camille500 / web-app-from-scratch Goto Github PK

The apiKey and request variable should be local

Route check in App.init() isn't necessary, do the same func. in routie

Personally I would still try to combine the instantiations of the objects. For instance, create the singletons all at the bottom of the script.

If you load op the SPA, click on best practices, refresh the website, you'll see the introduction page. If you then click on BP you wont navigate to that page because the SPA thinks you're already there.

Currently you've got some functions for adding and reacting to events. You could put everything in a events object en init the listeners on app.init().

Currently, you've got functions like formatMoney en randomNumber. It would be nice to have these functions as methods of an utils object

showData should be in the section object as method

My pull request is my advice in the form of an issue, otherwise I would re-write your code.

Currently the functionality is limited to two sections, old and new. You should get all the sections and check if the location.hash === section.id. This scales much better and is probably easier to read :).

Sidenote, use forEach for the looping through all the sections instead of a old for loop. A lot of people aren't using the awesome forEach loop yet

It's hard for other developers to understand your code if there is not some explanation about what is going on.

You always call movieData.cleanList(movieList) at the end of every if statement, you can just call it once at the end.

            filterList(movieList, type) {
                if (type === 'highest_rating') {
                    movieList.results.sort(function(a, b) {
                        return b.vote_average - a.vote_average;
                    });
                    movieData.cleanList(movieList)
                } else if (type === 'higher_rating') {
                    movieList.results = movieList.results.filter(function(movie) {
                        return movie.vote_average > 5.5;
                    });
                    movieData.cleanList(movieList)
                } else if (type === 'most_votes') {
                    movieList.results.sort(function(a, b) {
                        return b.vote_count - a.vote_count;
                    });
                    movieData.cleanList(movieList)
                } else if (type === 'most_popular') {
                    movieList.results.sort(function(a, b) {
                        return b.popularity.toFixed(1) - a.popularity.toFixed(1);
                    });
                    movieData.cleanList(movieList)
                }
},

Code comments for every method & function

getData shoudn't be an object, but an method of the app object

Cache all API data if it isn't necessary to do a call for