Create rankingService.js #33
Conversation
src/service/rankingService.js
Outdated
| currentMode.sort( (a,b) => b.timeInSeconds - a.timeInSeconds ); | ||
| currentMode.sort( (a,b) => b.score - a.score ); |
There was a problem hiding this comment.
Two separate sorts are bad - you cannot be sure that sorting is stable.
More on sort stability and what it even is: https://medium.com/@fsufitch/is-javascript-array-sort-stable-46b90822543f
It is better to make one sorting function, in example:
(a,b) => {
const scoreCompare = b.score - a.score;
if(scoreCompare != 0) { // if scores are different, then it is all we want.
return scoreCompare;
} else { // if score is the same, then we need to compare only time of those elements.
return b.timeInSeconds - a.timeInSeconds;
}
}
src/service/rankingService.js
Outdated
| currentMode = PokemonApiRanking.mode3.scores; | ||
| break; | ||
| default: | ||
| trow `Wrong mode`; |
There was a problem hiding this comment.
the best is to use throw new Error('message')
src/service/rankingService.js
Outdated
| // check localStorage for current Top scores | ||
| // if empty create object with objects mode1, mode2, and mode3 | ||
| // PokemonApiRanking - data from/to localStorage | ||
| const PokemonApiRanking = JSON.parse(localStorage.getItem('PokemonApiRanking')) || { |
There was a problem hiding this comment.
You should get it every time you call rankingService function, not only at the start.
If you once add score to localStorage then this PokemonApiRanking is updated, okay... but the second score that would be added is based not on the content of the local storage (that could be somehow changed between first and second game), but is based on the local copy you have in this variable. (Not sure if this is clear enough - if not - contact me)
src/service/rankingService.js
Outdated
| currentMode.push(user); | ||
| currentMode.sort( (a,b) => b.timeInSeconds - a.timeInSeconds ); | ||
| currentMode.sort( (a,b) => b.score - a.score ); | ||
| if(currentMode.length > 3) { | ||
| currentMode.length = 3; | ||
| } |
There was a problem hiding this comment.
this approach looks better for me
There was a problem hiding this comment.
but setting array length is not good 🤮
There was a problem hiding this comment.
best thing would be to:
- add new record to old array
- sort it
- create new array
- add only 3 first elements of old array to new array
- set "new array" as right property of whole PokemonApiRanking
src/service/rankingService.js
Outdated
| export const printTopUsers = (users = [], htmlList) => { | ||
| htmlList.innerHTML = users.map((user, i) => { | ||
| return ` | ||
| <li> | ||
| <label for="item${i}"><strong>${user.name}</strong>: ${user.score}points in ${user.timeInSeconds}seconds</label> | ||
| </li> | ||
| `; | ||
| }).join(''); | ||
| } |
There was a problem hiding this comment.
displaying records and whole rankingService functionality should be in separate files (separation of responsibilities)
some small changes and: - removed print to html function - created update from localStorage function
No description provided.