Jawad A.#15
Conversation
📝 HackYourFuture auto gradeAssignment Score: 0 / 100 ✅Status: ✅ Passed Test Details |
mo92othman
left a comment
mo92othman
left a comment
There was a problem hiding this comment.
Hey @jivvyjams , thanks for the submission 👍
Good job! The overall structure is clear!
Your code still doesn't work for a few small reasons. I left a few comments. Let me know if something is not clear!
| export function getBalance() { | ||
| return getTotalIncome() - getTotalExpenses(); | ||
| } |
There was a problem hiding this comment.
There is a logic issue in getBalance
getTotalIncome and getTotalExpenses require transactions,
but they are called here without passing any arguments.
| export function printSummary(transactions) { | ||
| const totalIncome = getTotalIncome(transactions); | ||
| const totalExpenses = getTotalExpenses(transactions); | ||
| const balance = getBalance(transactions); |
There was a problem hiding this comment.
Because getBalance does not accept parameters, the balance calculation here will not be correct.
| @@ -1,3 +1,14 @@ | |||
| // This is the entrypoint for your application. | |||
| // node app.js | |||
| import { transactions } from './data.js'; | |||
There was a problem hiding this comment.
This import here will not work because of the module styles: CommonJS. Check my comment on the package.json file.
| "keywords": [], | ||
| "author": "", | ||
| "license": "ISC", | ||
| "type": "commonjs", |
There was a problem hiding this comment.
You mix two module styles: CommonJS and ES modules. If you use commonjs you need to use require.
Or you can enable ES modules ("type": "module"), and then also imports must match how values are exported.
For example, in data.js, which uses a default export, it should be imported without curly braces:
import transactions from './data.js';
| }, | ||
| ]; | ||
|
|
||
| export default transactions; |
There was a problem hiding this comment.
data.js uses a default export, so it should be imported without {}.
| import { | ||
| addTransaction, | ||
| getTotalIncome, | ||
| getTotalExpenses, | ||
| getBalance, | ||
| getTransactionsByCategory, | ||
| getLargestExpense, | ||
| printAllTransactions, | ||
| printSummary, | ||
| } from './finance.js'; |
There was a problem hiding this comment.
Small suggestion 🙂
Some imported functions are not used in this file.
It’s best practice to remove unused imports to keep the code clean and easier to read.
mo92othman
added
Reviewed
2 participants
No description provided.