Skip to content

Magnus Krumbacher#120

Open
Magnus-droid wants to merge 12 commits intoboolean-uk:mainfrom
Magnus-droid:main
Open

Magnus Krumbacher#120
Magnus-droid wants to merge 12 commits intoboolean-uk:mainfrom
Magnus-droid:main

Conversation

@Magnus-droid
Copy link

No description provided.

Copy link

@ninja-aruna ninja-aruna left a comment

Choose a reason for hiding this comment

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

Good work so far, a few comments added to help you out (maybe)

this.stringbuilder.insert(0, String.format("%21s || %10s || %8s || %8s\n", "Date", "Withdrawal", "Deposit", "Balance"));
}

public void makeTransaction(double amount) {

Choose a reason for hiding this comment

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

I would recommend using a transactiontype enum to figure out whether to do a deposit or withdrawal. It does not seem right to have a makeTransaction method also generate a statement. Consider having a getBalance method and call that in a new function that is only responsible for generating statements

public class Transaction {
private final String date; //needs time as well!!
private double amount;
private final String type;

Choose a reason for hiding this comment

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

Strongly typed (enum) would be better here, as strings can have typos :)

return date;
}

//Don't want other classes to be able to set the type so this is done internally

Choose a reason for hiding this comment

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

Good!

@Magnus-droid
Copy link
Author

I tried to use an interface in order to practice using it, but I think it ended up being more work than if I had not. Do you have any pointers on what I could do differently when it comes to using an interface for this task? Apart from that, I think the enum type works now but I had to make it public in the Transaction class which I don't think is good. I could not get the generateStatement() method to build the transaction history properly without updating it when a transaction is made :(

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