Conversation
This comment has been minimized.
This comment has been minimized.
reposman33
left a comment
reposman33
left a comment
There was a problem hiding this comment.
lots of comments... you try to be complete (which is good) but don't overdo. And chaining array methods (or any method that returns an array like loadBooks()) is an easy catch
| const filePath = path.join(dataDir, 'books.json'); | ||
| const defaultArray = []; | ||
|
|
||
| function isDataFull(book) { |
There was a problem hiding this comment.
{id:'2' title:'', author: '', genre:''} is also dataFull but maybe not what you mean...You would have to check if the properties values are actually valid (not '', id is not '2' but 2 etc) Nice that you want to validate the book object but it is not a requiremwent here so I would just not do it.
| // Use try-catch for error handling | ||
| export function loadBooks() { | ||
| try { | ||
| const data = JSON.parse(fs.readFileSync(filePath, 'utf-8')); |
There was a problem hiding this comment.
if you check with fs.existsSync() if the file exists before reading you can catch that as well, just like you check the !Array.isArray(data)) case.
| } catch (error) { | ||
| if (error.code === 'ENOENT') { | ||
| console.log('Books file not found'); | ||
| fs.writeFileSync(filePath, JSON.stringify(defaultArray, null, 2)); |
There was a problem hiding this comment.
defaultArray === [] so JSON.stringify(defaultArray, null, 2) needs no 2. Could be even fs.writeFileSync(filePath, JSON.stringify(defaultArray));
| export function saveBooks(books) { | ||
| try { | ||
| if (!Array.isArray(books)) { | ||
| throw new Error("Books file doesn't contain an array"); |
There was a problem hiding this comment.
you are right to say 'Books file doesn't contain an array' but a common user doesn't know what an array is. I would say something like 'Books file doesn't contain valid content'
| } | ||
|
|
||
| if (!books.every((book) => isDataFull(book))) { | ||
| throw new Error('Book must has id, title, author and genre'); |
There was a problem hiding this comment.
ah, that's what isDataFull is for.... kudos for the check. But not really needed here I think. Being lazy isn't always bad ;)
| const books = loadBooks(); | ||
|
|
||
| if (books.some((item) => item.id === book.id)) { | ||
| throw new Error(`Book with ${book.id} already exists`); |
There was a problem hiding this comment.
Good catch... it could be there already.
| // TODO: Implement this function using filter() | ||
| // console.log('Get unread books:', getUnreadBooks()); | ||
|
|
||
| export function getBooksByGenre(genre) { |
There was a problem hiding this comment.
Array methods can be chained. So this can be:
getBooksByGenre() =>
loadBooks().filter((book) => book.genre === genre)
| const books = loadBooks(); | ||
| const updateForRead = books.map((book) => { | ||
| if (book.id === id) { | ||
| return { ...book, read: true }; |
There was a problem hiding this comment.
well done... create a new object from the original and just overwrite the read property
| // TODO: Implement this function using some() | ||
| // console.log('Total books:', getTotalBooks()); | ||
|
|
||
| export function hasUnreadBooks() { |
There was a problem hiding this comment.
can.be.chained: loadBooks().books.some((book) => !book.read);
| { | ||
| "name": "c55-core-week-6", | ||
| "version": "1.0.0", | ||
| "description": "The week 6 assignment for the HackYourFuture Core program can be found at the following link: https://hub.hackyourfuture.nl/core-program-week-6-assignment", |
There was a problem hiding this comment.
Nice... this helps the user of your app
reposman33
added
Reviewed
… remove redundant validation, and apply array method chaining
📝 HackYourFuture auto gradeAssignment Score: 0 / 100 ✅Status: ✅ Passed Test Details |
|
Hi! Thank you for the feedback — it was really helpful. I went through all your comments and updated the code accordingly:
I also refactored loadBooks() so it doesn’t overwrite the JSON file when an error occurs, but still normalizes an empty file to [] to keep the data format consistent for the rest of the functions. This prevents data loss while keeping the module stable. Thanks again for the review — it definitely helped make the code cleaner and more robust. |
2 participants
Build a command-line reading list that saves to files using error handling and array methods.