Yusup R. - #10
Yusup R.#10Yusuprozimemet wants to merge 2 commits into
Conversation
📝 HackYourFuture auto gradeAssignment Score: 0 / 100 ✅Status: ✅ Passed Test Details |
rafaelhdr
left a comment
rafaelhdr
left a comment
There was a problem hiding this comment.
Good job in this exercise 👏
Your code is very easy to read. I had just some small suggestions.
| SELECT b.title, b.published_year | ||
| FROM books b | ||
| JOIN authors a ON a.id = b.author_id | ||
| WHERE a.first_name || ' ' || a.last_name = 'Stephen King' |
There was a problem hiding this comment.
Here is not necessary to create the author full name. AND would be better, because databases could even use some performance improvements
- WHERE a.first_name || ' ' || a.last_name = 'Stephen King'
+ WHERE a.first_name = 'Stephen' AND a.last_name = 'King'
| const aSingleDeck = db.prepare('SELECT * FROM decks WHERE id = ?').get(id); | ||
| if (aSingleDeck) { | ||
| return aSingleDeck; | ||
| } else { |
There was a problem hiding this comment.
It is good, but you don't need the else here. It is fine tho, because this is small, but it would be nice to take a quick look in the pattern of early return.
https://dev.to/eddiegoldman/early-return-vs-classic-if-else-a-universal-pattern-for-writing-cleaner-code-1083 (this was the first result of a quick search)
But it is something nice. When you have many if ... else if ... else, it makes your code more clean :)
| // Insert all cards from JSON into database | ||
| const allCardsFromJson = parsedJsonData.cards; | ||
| for (const eachCard of allCardsFromJson) { | ||
| let learnedValue; |
There was a problem hiding this comment.
It works great the way it is 👍
But you could use ternary here.
const learnedValue === true ? 1 : 0;
And it would have the same result
rafaelhdr
added
Reviewed
2 participants
No description provided.