[CS2113-W14-3] FinanceBuddy #25

limkongkiat · 2024-09-30T11:57:33Z

Finance Buddy is a software that allows university students to log their daily expenditures to help manage their budgets. Users can add, delete and edit expenditure logs into the app as well as list out all their logged transactions.

thomasjlalba

Some areas can be simplified to make the logic and flow more obvious and simpler to read. Keep up the good work!

thomasjlalba · 2024-10-25T08:01:57Z

src/main/java/seedu/duke/command/AddIncomeCommand.java

+        System.out.println("--------------------------------------------");
+        System.out.println("Got it! I've added this income:");
+        System.out.println(income);
+        System.out.println("--------------------------------------------");


might be good to refactor this such that it is not magic strings

thomasjlalba · 2024-10-25T08:08:26Z

src/main/java/seedu/duke/command/DeleteCommand.java

+            FinancialEntry entry = list.getEntry(index - 1);
+            list.deleteEntry(index - 1);  // Index correction as list is 0-based


Maybe you can refactor index - 1 as a variable since it is used multiple times, and also to make it clearer that it is because list is 0-based

thomasjlalba · 2024-10-25T08:15:09Z

src/main/java/seedu/duke/command/DeleteCommand.java

+        if (index > 0 && index <= list.getEntryCount()) {
+            assert index > 0 && index <= list.getEntryCount();


Since it is used multiple times, you could refactor the condition statement as a boolean variable

thomasjlalba · 2024-10-25T08:17:30Z

src/main/java/seedu/duke/command/DeleteCommand.java

+        } else {
+            System.out.println("OOPS!!! The entry does not exist.");


You can make the happy path more prominent by making this a guard clause instead

thomasjlalba · 2024-10-25T08:19:17Z

src/main/java/seedu/duke/command/SeeAllIncomesCommand.java

+    private LocalDate start;
+    private LocalDate end;


The variable name might be a bit ambiguous, maybe it could be better refined as startDate?

thomasjlalba · 2024-10-25T08:36:25Z

src/main/java/seedu/duke/financial/FinancialList.java

+        } else {
+            System.out.println("OOPS!!! The entry does not exist.");
+            return null;
+        }


Maybe this can be converted into an error?

thomasjlalba · 2024-10-25T08:45:54Z

src/main/java/seedu/duke/ui/AppUi.java

+     *                         and the amount ("/a") and the date/time ("/dt")
+     */
+    public void addExpense(HashMap<String, String> commandArguments) {
+        String description = commandArguments.get("argument");


Might be good to refactor these magic strings

thomasjlalba · 2024-10-25T08:48:10Z

src/main/java/seedu/duke/ui/AppUi.java

+        String start = commandArguments.get("/from");
+        String end = commandArguments.get("/to");


Maybe you can better name these variables to startDateString, as it might be confusing alongside startDate and endDate.

thomasjlalba · 2024-10-25T08:49:00Z

src/test/java/seedu/duke/command/AddExpenseCommandTest.java

+        addExpenseCommand.execute(financialList);
+
+        String output = outputStream.toString();
+        String expectedOutput = "--------------------------------------------" + System.lineSeparator() +


Maybe can keep the consistency between \n and System.lineSeperator()

thomasjlalba · 2024-10-25T08:52:06Z

src/main/java/seedu/duke/parser/InputParser.java

+     * @param input The raw input string from the user.
+     * @return A HashMap containing the parsed command and its arguments.
+     */
+    public static HashMap<String, String> parseCommands(String input) {


Maybe you can look to splitting this method to multiple simpler methods

ZiliaAJY

Overall good job!

ZiliaAJY · 2024-10-30T02:27:36Z

docs/UML/CommandInheritance.png

I found quite a few rounded rectangles, consider using normal rectangle boxes.

ZiliaAJY · 2024-10-30T02:31:19Z

docs/UML/SeeAllEntriesExecution.png

Objects are missing semi-colons, for example :Logic (in quite a few sequence diagrams).

ZiliaAJY · 2024-10-30T02:33:28Z

docs/UML/SeeAllEntriesOverview.png

The parameters are missing the corresponding types.

ZiliaAJY · 2024-10-30T02:35:33Z

docs/UML/SeeAllEntriesOverview.png

Certain conditions in 'alt' can be indicated as "else".

telferang

The diagram is mostly correct. I added some comments for your reference.

telferang · 2024-10-30T02:26:39Z

docs/DeveloperGuide.md

+The diagram below shows the inheritance of the `Command` class. The diagram is only meant to show
+the hierarchy of classes and have been greatly simplified.
+
+![Command_Inheritance_Diagram](UML/CommandInheritance.png)


Consider using a normal rectangle instead of a rounded rectangle

telferang · 2024-10-30T02:29:10Z

docs/DeveloperGuide.md

+The diagram below shows the inheritance of the `Command` class. The diagram is only meant to show
+the hierarchy of classes and have been greatly simplified.
+
+![Command_Inheritance_Diagram](UML/CommandInheritance.png)


Consider switching the direction of the arrow

telferang · 2024-10-30T02:29:36Z

docs/DeveloperGuide.md

+The diagram below shows the inheritance of the `Command` class. The diagram is only meant to show
+the hierarchy of classes and have been greatly simplified.
+
+![Command_Inheritance_Diagram](UML/CommandInheritance.png)


Consider using an empty arrow instead of a filled arrow

telferang · 2024-10-30T02:37:28Z

docs/DeveloperGuide.md

+The interaction between the command classes and the `FinancialList` is as follows,
+using `SeeAllEntriesCommand` as an example:
+
+![execution](UML/SeeAllEntriesExecution.png)


Consider adding [else] for the alt frame

telferang · 2024-10-30T02:38:48Z

docs/DeveloperGuide.md

+The interaction between the command classes and the `FinancialList` is as follows,
+using `SeeAllEntriesCommand` as an example:
+
+![execution](UML/SeeAllEntriesExecution.png)


Consider adding a ":" before an entities

hzxnancy

Overall, its a really good DP, as each command have a lot of details and the DP is structured neatly! Just take note of the minor details and should be alright.

hzxnancy · 2024-10-30T02:26:35Z

docs/UML/SeeAllEntriesOverview.png

Generally a well drawn diagram, but would be better if there are activation bars!

hzxnancy · 2024-10-30T02:43:43Z

docs/UML/SeeAllEntriesExecution.puml

+participant "SeeAllEntriesCommand" as cmd
+participant "FinancialList" as list
+participant "FinancialEntry" as entry


Would it be better to start the entity's name with a colon?

hzxnancy · 2024-10-30T03:26:56Z

docs/UML/overallFlow.png

May I ask if this is an activity diagram? If it is, it would be better to use coloured cirlces to denote the start and end of the program!

Toby-Yu

Overall DG is ok but there is something needs to be added

Toby-Yu · 2024-10-30T04:37:30Z

docs/UML/SeeAllEntriesExecution.png

The sequence diagram seems miss the activation bar.

Toby-Yu · 2024-10-30T04:38:23Z

docs/UML/overallFlow.png

This structural diagram is clear.

Toby-Yu · 2024-10-30T04:41:52Z

.gitignore

Is good to put data into the gitignore

Toby-Yu · 2024-10-30T04:44:05Z

docs/DeveloperGuide.md

There is no example of input and output screenshots for the explanation with the DG.

Update PPP

Update UG post ped

Add-storage-feedback

Bugfix edit out of position

…-Time-Feature Update DateParser

Update PPP

Developer Guide

Update DG final

Ug duplicate

update

Update UserGuide.md

update

Update PPP

Developer guide

User guide

Update UG

thomasjlalba reviewed Oct 25, 2024

View reviewed changes

ZiliaAJY reviewed Oct 30, 2024

View reviewed changes

telferang reviewed Oct 30, 2024

View reviewed changes

hzxnancy reviewed Oct 30, 2024

View reviewed changes

Toby-Yu reviewed Oct 30, 2024

View reviewed changes

tsoudibi and others added 25 commits November 10, 2024 02:48

Fix checkStyle

6daeb24

Update DateParser

8a948b1

Edit UG

17ce099

Add no / in description

c6dd537

Update date format dd/MM/yyyy

a653485

Update year to yyyy

d95a584

Update PPP

008c5eb

Merge pull request #280 from Littleme1032/Littleme1032-Juven-edit-ppp

176886f

Update PPP

Merge branch 'master' into Budget

1e0f67d

Update budget prompting to user on startup

1f00a8c

Add line breaks in Help sample output

f27b49a

Merge pull request #275 from limkongkiat/update-UG-post-ped

8315007

Update UG post ped

Update when balance recalculation is done

dab656e

Prevent budget amount from exceeding 9 999 999

9256451

Minor refactoring of method name to more appropriately suit what it does

40ff2e3

Update JUnit tests for added functionality

7ef1e6c

Update JavaDoc for new methods and JUnit tests

de52b72

Merge pull request #277 from tsoudibi/add-storage-feedback

9380945

Add-storage-feedback

Update edit UG

6cab419

Seperate SD into 2

024bc80

Merge branch 'master' into Littleme1032-juven-add-Date-Time-Feature

4c7e172

Edit test case

92d8484

Merge pull request #276 from limkongkiat/bugfix-edit-out-of-position

08b3301

Bugfix edit out of position

Edit UG

3083d81

Merge pull request #279 from Littleme1032/Littleme1032-juven-add-Date…

73fd350

…-Time-Feature Update DateParser

limkongkiat and others added 30 commits November 12, 2024 13:03

Update DG

03e5102

Update Diagrams

b7a4d05

Merge pull request #335 from ctzeyong/PPP

7f36391

Update PPP

Merge branch 'master' into DeveloperGuide

e6a58ff

Update add transaction

e47c51d

Update DG UML

e6eea66

update

d5039a9

Add Page Break to DG

6f18e97

Merge pull request #337 from ctzeyong/DeveloperGuide

80b58bd

Developer Guide

Remove extra colon in Contents Page

ea2e0d1

Merge pull request #338 from limkongkiat/update-DG-final

be2e13c

Update DG final

Merge pull request #336 from Littleme1032/UG-duplicate

cb92e18

Ug duplicate

update

7cbd9eb

Merge pull request #339 from Littleme1032/UG-duplicate

9a57cfd

update

Update UserGuide.md

01135ba

Merge pull request #340 from Littleme1032/UG-duplicate

8dcc1b9

Update UserGuide.md

update

2166909

Merge pull request #341 from Littleme1032/Ug-update

af61259

update

update

acacb64

Merge pull request #342 from Littleme1032/Ug-update

3f92d65

update

Update PPP

b383b4b

Merge branch 'master' into DeveloperGuide

a65d50f

Merge pull request #343 from limkongkiat/final-update-ppp

a99f9ac

Update PPP

Fix broken links

4f997a0

Merge pull request #344 from ctzeyong/DeveloperGuide

817ddfe

Developer guide

Merge branch 'master' into UserGuide

b7776fb

Minor bug fix

52eaf5f

Merge pull request #345 from ctzeyong/UserGuide

b3a1fb8

User guide

Update UG

a28d310

Merge pull request #346 from limkongkiat/last-update-ug

2af99fb

Update UG

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[CS2113-W14-3] FinanceBuddy #25

[CS2113-W14-3] FinanceBuddy #25

limkongkiat commented Sep 30, 2024

thomasjlalba left a comment

thomasjlalba Oct 25, 2024

thomasjlalba Oct 25, 2024

thomasjlalba Oct 25, 2024

thomasjlalba Oct 25, 2024

thomasjlalba Oct 25, 2024

thomasjlalba Oct 25, 2024

thomasjlalba Oct 25, 2024

thomasjlalba Oct 25, 2024

thomasjlalba Oct 25, 2024

thomasjlalba Oct 25, 2024

ZiliaAJY left a comment

ZiliaAJY Oct 30, 2024

ZiliaAJY Oct 30, 2024 •

edited

Loading

ZiliaAJY Oct 30, 2024

ZiliaAJY Oct 30, 2024

telferang left a comment

telferang Oct 30, 2024

telferang Oct 30, 2024

telferang Oct 30, 2024

telferang Oct 30, 2024

telferang Oct 30, 2024

hzxnancy left a comment

hzxnancy Oct 30, 2024

hzxnancy Oct 30, 2024

hzxnancy Oct 30, 2024

Toby-Yu left a comment

Toby-Yu Oct 30, 2024

Toby-Yu Oct 30, 2024

Toby-Yu Oct 30, 2024

Toby-Yu Oct 30, 2024

		FinancialEntry entry = list.getEntry(index - 1);
		list.deleteEntry(index - 1); // Index correction as list is 0-based

		if (index > 0 && index <= list.getEntryCount()) {
		assert index > 0 && index <= list.getEntryCount();

		} else {
		System.out.println("OOPS!!! The entry does not exist.");

		String start = commandArguments.get("/from");
		String end = commandArguments.get("/to");

[CS2113-W14-3] FinanceBuddy #25

Are you sure you want to change the base?

[CS2113-W14-3] FinanceBuddy #25

Conversation

limkongkiat commented Sep 30, 2024

thomasjlalba left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ZiliaAJY left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ZiliaAJY Oct 30, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

telferang left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hzxnancy left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Toby-Yu left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ZiliaAJY Oct 30, 2024 •

edited

Loading