Skip to content

Will Baxter#55

Open
MrStashy wants to merge 15 commits into
boolean-uk:mainfrom
MrStashy:main
Open

Will Baxter#55
MrStashy wants to merge 15 commits into
boolean-uk:mainfrom
MrStashy:main

Conversation

@MrStashy

Copy link
Copy Markdown

No description provided.

Comment thread spec/Accounts.spec.js
Comment on lines +65 to +67
expect(
testAccount.getTransactions("2023-9-4", "2023-9-10").length
).toEqual(3);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this, but I think you should also have some expects for each of the transaction dates to ensure the correct transactions are being returned

Comment thread src/Accounts.js
Comment on lines +17 to +27
credit(amount, testDate) {
if (this.constructor.name === "SavingsAccount") {
const today = new Date();
const oneYearAgo = today - 365 * 24 * 60 * 60 * 1000;
const totalCreditsThisYear = this.getTransactions(oneYearAgo, today)
.filter((transaction) => transaction.constructor.name === "Credit")
.reduce((a, b) => a + Number(b.amount), 0);
if (totalCreditsThisYear + amount > 20000) {
throw new Error("You are only able to deposit £20,000 per year");
}
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice solution!

Comment thread src/Accounts.js
Comment on lines +47 to +54
getDate(testDate) {
if (testDate) {
const date = new Date(testDate);
return date;
}
const date = new Date();
return date;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if (testDate) { // why is it called testDate?
  return new Date(testDate)
}

return new Date()

Comment thread src/Accounts.js
Comment on lines +133 to +136
const filteredTransactions = this.#transactions.filter(
(transaction) => transaction.constructor.name === "Credit"
);
return [...filteredTransactions];

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

filter will return a new array containing the filtered items so you're safe to return it directly

return this.#transactions.filter(
      (transaction) => transaction.constructor.name === "Credit"
    )

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants