Conversation
📝 HackYourFuture auto gradeAssignment Score: 0 / 100 ✅Status: ✅ Passed Test Details |
reposman33
left a comment
reposman33
left a comment
There was a problem hiding this comment.
Some comments nd one function needs fixing: function getLargestExpense()
| // LARGEST EXPENSE & COUNT | ||
| const largest = getLargestExpense(); | ||
| console.log( | ||
| `\nLargest Expense: ${chalk.yellow(largest.description)} (${chalk.red('€' + largest.amount)})`, |
There was a problem hiding this comment.
in backticks you just can use Return, you don't have to use \n. So
console.log(
\nLargest Expense: ${chalk.yellow(largest.description)} (${chalk.red('€' + largest.amount)})
becomes
console.log( Largest Expense: ${chalk.yellow(largest.description)} (${chalk.red('€' + largest.amount)})
There was a problem hiding this comment.
Thank you for the detailed feedback. I realize there were several areas for improvement, specifically regarding object initialization, using built-in array methods like .filter(), removing unnecessary \n escape characters in template literals, and overall code formatting.
I am reviewing all the lessons now and will refactor the code to be more concise and professional. I will resubmit the corrected version with Prettier formatting applied very soon.
| `\nLargest Expense: ${chalk.yellow(largest.description)} (${chalk.red('€' + largest.amount)})`, | ||
| ); | ||
| // Use the .length property of the transactions array we imported | ||
| const transactions = require('./data'); |
There was a problem hiding this comment.
Put all imports (require()) at the beginning for clarity
| function getTotalIncome() { | ||
| // TODO: Implement this function | ||
| let total = 0; | ||
| for (const tx of transactions) { |
There was a problem hiding this comment.
This works fine. A shorter way to reduce an array to a number is to use the reduce function():
const total = transactions.reduce().
|
|
||
| function getTotalExpenses() { | ||
| // TODO: Implement this function | ||
| let total = 0; |
There was a problem hiding this comment.
This works fine. A shorter way to reduce an array to a number is to use the reduce function():
const total = transactions.reduce().
|
|
||
| function getTransactionsByCategory(category) { | ||
| // TODO: Implement this function | ||
| const filtered = []; |
There was a problem hiding this comment.
This works fine. A shorter way to filter elements from an array is to use the filter function():
const filtered = transactions.filtered().
| for (const tx of transactions) { | ||
| // Check if it is an expense AND if it's larger than our current record | ||
| if (tx.type === 'expense' && tx.amount > largest.amount) { | ||
| largest = tx; |
There was a problem hiding this comment.
this works but it is a bit confusing... why do you first declare
let largest = { amount: 0, description: 'None' };
and in the loop you overwrite largest and have it point to tx;? What do we need { amount: 0, description: 'None' };
for? If you overwrite it anyway, largest can also be 1, or null or false.
Defining largest = { amount: 0, description: 'None' }; makes me expect that aftr the function largest will be { amount: 1200, description: 'rent' }.
Either define largest as null (like Shadi did) or even better, keep largest but make sure that largest.amount and largest description point to the greatest expense after running the function
There was a problem hiding this comment.
Thank you for the feedback. I will take the correction regarding the object initialization and format the entire codebase with Prettier. I will resubmit it as soon as possible.
reposman33
added
Reviewed
| "url": "https://github.com/Dagim02/c55-core-week-4/issues" | ||
| }, | ||
| "homepage": "https://github.com/Dagim02/c55-core-week-4#readme", | ||
| "dependencies": { |
There was a problem hiding this comment.
I'm missing prettier...Format entire codebase with Prettier before submission
2 participants
No description provided.