[CS2113-T18-2] FinancialPlanner by ryan1604 · Pull Request #17 · nus-cs2113-AY2324S1/tp

ryan1604 · 2023-10-05T09:22:04Z

Financial Planner helps individuals manage their finances effectively and achieve their financial goals. The purpose of such an application is to provide users with a range of tools and features to help them better understand their financial situation.

Make format of flashcard Directory

maanyos

Good use of loggers and assertions.

I think you tried to apply the singleton principle for some classes. It will be covered in one of the lectures, you can also read ahead under week 10's topics. The current implementation can improve a lot, particularly the instance names, and the way they are accessed.

maanyos · 2023-10-22T07:14:57Z

@@ -0,0 +1,5 @@
+package seedu.financialplanner.commands;
+
+public abstract class AbstractCommand {


no need to put abstract in the class name

maanyos · 2023-10-22T07:24:52Z

+    private static void loadBudget(String[] split) {
+        double initial = Double.parseDouble(split[1].trim());
+        double current = Double.parseDouble(split[2].trim());
+        Budget.load(initial, current);
+    }


might want to add some safeguards here

maanyos · 2023-10-22T07:25:18Z

+                String[] split = line.split("\\|");
+                String type = split[0].trim();
+                switch (type) {
+                case "I":


is this supposed to be empty?

maanyos · 2023-10-22T07:28:08Z

+    public BudgetCommand(RawCommand rawCommand) throws FinancialPlannerException {
+        command = String.join(" ", rawCommand.args);
+        if (!command.equals("set") && !command.equals("update")) {
+            logger.log(Level.WARNING, "Invalid arguments for budget command");
+            throw new FinancialPlannerException("Please indicate whether budget is to be set or update.");
+        }
+
+        if (command.equals("set") && Budget.hasBudget()) {
+            logger.log(Level.WARNING, "Invalid command: Trying to set existing budget");
+            throw new FinancialPlannerException("There is an existing budget, did you mean update?");
+        }
+
+        if (command.equals("update") && !Budget.hasBudget()) {
+            logger.log(Level.WARNING, "Invalid command: Trying to update non-existent budget");
+            throw new FinancialPlannerException("There is no budget set yet, did you mean set?");
+        }
+
+        if (!rawCommand.extraArgs.containsKey("b")) {
+            logger.log(Level.WARNING, "Missing argument /b in command");
+            throw new IllegalArgumentException("Missing /b argument.");
+        }
+
+        try {
+            logger.log(Level.INFO, "Parsing budget as double");
+            budget = Double.parseDouble(rawCommand.extraArgs.get("b"));
+        } catch (IllegalArgumentException e) {
+            logger.log(Level.WARNING, "Invalid value for budget");
+            throw new IllegalArgumentException("Budget must be a number.");
+        }
+
+        if (budget <= 0) {
+            logger.log(Level.WARNING, "Invalid value for budget.");
+            throw new FinancialPlannerException("Budget should be greater than 0.");
+        }
+
+        if (budget > Cashflow.getBalance()) {
+            logger.log(Level.WARNING, "Invalid value for budget");
+            throw new FinancialPlannerException("Budget should be lower than total balance.");
+        }
+
+        assert budget > 0 && budget <= Cashflow.getBalance() : "Budget should be greater than 0 and less than " +
+                "or equal to total balance";
+        rawCommand.extraArgs.remove("b");
+
+        if (!rawCommand.extraArgs.isEmpty()) {
+            String unknownExtraArgument = new ArrayList<>(rawCommand.extraArgs.keySet()).get(0);
+            throw new IllegalArgumentException(String.format("Unknown extra argument: %s", unknownExtraArgument));
+        }
+    }


try using a neater way to put safeguards

maanyos · 2023-10-22T07:30:53Z

+    public static final CashflowList INSTANCE = new CashflowList();
+    public final ArrayList<Cashflow> list = new ArrayList<>();


use better names for these lists

maanyos · 2023-10-22T07:44:28Z

+import java.util.List;
+import java.util.Map;
+
+public class RawCommand {


how does this class relate to the other commands?

maanyos · 2023-10-22T07:52:19Z

+        for (int i = 0; i < cashflowList.list.size(); i++) {
+            if (cashflowList.list.get(i).toString().contains(description)) {
+                String output = cashflowList.list.get(i).toString()+" | Index: "+(i+1);
+                foundedFinancialList.add(output);
+            }
+        }


can also just iterate through the objects instead of using their indexes

Delete Reminder and display reminder

woodenclock

Generally good job in making a comprehensive developer guide.
I really like your visualizer, that enhances the UI/UX.
Diagrams were present to support the understanding of the program

woodenclock · 2023-11-01T03:22:50Z

@@ -1,13 +1,249 @@
 # Developer Guide

+## Table of contents


Good use of indentation to organize the Table of Contents.
However, since there are 3 types of indentation, could there be a better way of labelling each line? say 1.1.1?
Just a small suggestion.

woodenclock · 2023-11-01T03:24:55Z

+  * [Glossary](#glossary)
+  * [Instructions for manual testing](#instructions-for-manual-testing)
+
 ## Acknowledgements


Comprehensive acknowledgement of others.
Properly citing third-party libraries and tools ensures transparency and gives credit to the original creators.
This section showcases the developer's integrity and understanding of open-source norms.

woodenclock · 2023-11-01T03:31:49Z

+
 {list here sources of all reused/adapted ideas, code, documentation, and third-party libraries -- include links to the original source as well}

 ## Design & implementation


Perhaps it would be good to have a Architecture Diagram showcasing your overall design and mechanism of FinancialPlanner. This would give the Developer their needed context should they want to further implement other features to integrate into your program

woodenclock · 2023-11-01T03:33:41Z

+  * Advantage: Changes are saved after every command.
+  * Disadvantage: Executing command might slow down the program when there is a large amount of data to be saved.
+
+### Visualization Feature 


Kudos for integrating Xchart!
By having the visual aids, it would definitely allow the user to be aware of their expenses at a glance.
Even though FinancialPlanner is CLI, I like the GUI component added to enhance the UI/UX

woodenclock · 2023-11-01T03:35:09Z


 {Describe the design and implementation of the product. Use UML diagrams and short code snippets where applicable.}

+### Storage Component


The relationship between Storage and SaveData/LoadData seems slightly confusing.
Typically, if Storage is a class providing functionalities like saving and loading, it may have methods for those, rather than being connected to abstract classes representing those actions.
Consider redefining these relationships for clarity.

A clearer distinction or some elaboration on what differentiates Cashflow and Budget would be beneficial.

woodenclock · 2023-11-01T03:36:13Z

+According to the chart type (Pie/Bar) argument and the Hashmap obtained from the categorizer passed in, 
+the visualizer displays the specified visualization chart by calling the charting library Xchart.
+
+### Class Diagram


Visualizer seems to have methods that return some kind of chart, but the return types are not indicated.
It would be clearer if return types were included.

The relationship between Categorizer and other classes is not immediately evident.
Perhaps some connecting lines or indications of method calls/dependencies would be helpful.

VIsualizer would do the showing of chart itself. Thus it is a void method call to visualizer to print the chart.

Thanks for the feedback on the class diagram!
Will work on that!

woodenclock · 2023-11-01T03:37:14Z

+
+![](images/vis/visualisationClass.png)
+
+### Sequence Diagram 


In the sequence diagram, it would be beneficial to include return arrows, especially for method calls that yield some kind of result.
The method printDisplayChart(type) is invoked on Ui from VisCommand, but from the class diagram, it seems like the method belongs to the Visualizer class. This discrepancy should be resolved for consistency.
Or am I just confusing myself?

Thanks for the feedback! I can see the confusion. printDisplayChart prints a message telling the user that a chart is being displayed. Visualizer has a method for displaying the actual chart. I have renamed the method for UI to make it clearer. I will add the return arrows as well!

danielpappa

Overall great DG! Very detailed description and plenty of diagrams and code snippets to transmit the content in an intuitive way. Maybe could try to make it more coherent by paying attention to how each detail is actually formatted (colors, bullet points etc.). Kudos also for the use of diagrams!

danielpappa · 2023-11-01T04:26:27Z

Overall very good DG, very precise and in depth analysis of the code and detailed description.
Maybe could use a single and coherent way of using bullet points (right now the DG contains >>, 1), 1. and Step 1). This could make the DG look as if actually written by a single person.

danielpappa · 2023-11-01T04:27:53Z

 ## Acknowledgements

+**Xchart (A Simple Charting Library for Java)**
+- author: KNOWN


Maybe there is a typo here: shouldn't it be author: KNOWM?

THanks for spotting!

danielpappa · 2023-11-01T04:30:21Z


 ## Design & implementation

 {Describe the design and implementation of the product. Use UML diagrams and short code snippets where applicable.}


Maybe you could add a brief introduction and overview of your Financial Planner, explaining the goal and purpose as well as its rough overall design in form of an arbitrary architecture diagram. This would also allow further developers an immediate insight into your product and align them with your ideas and implementations from the start.

danielpappa · 2023-11-01T04:32:44Z

+* Saving the data once upon exit (Currently implemented):
+  * Advantage: Better efficiency and performance of the program.
+  * Disadvantage: If the program crashes or exits incorrectly, data will not be saved.
+* Saving the data after every command:


Perhaps it should be specified whether this way is currently implemented or not (as is done for the other option) to keep the considerations coherent. Right now I assume it is not, but I would still not be sure.

danielpappa · 2023-11-01T04:35:52Z

+Output
+
+`Displaying piechart for expense`
+A message will be shown telling you that the chart is being displayed


Really nice implementation. This feature is straightforward and meets the user requirements.

danielpappa · 2023-11-01T04:40:26Z

+
+### Class Diagram
+
+![](images/vis/visualisationClass.png)


In the text above Command and RawCommand are omitted, whereas it seems like there should be a dependency between the Categorizer and VisCommand. Maybe some more detailed steps could help clarify these doubts.

Thanks for your feedback, will add it in!

danielpappa · 2023-11-01T04:43:23Z

+
+Overall 
+
+![](images/vis/visualisationSequence.png)


Maybe you could include return arrows. Also it could perhaps be helpful to make the alt and opt boxes coherent by applying the coloring to all sequence diagrams in the DG (helps create impression that DG was written by a single person).

ChoonSiang

Overall very nice use of diagrams, except some minor bugs in the UML diagrams

ChoonSiang · 2023-11-01T04:33:15Z

+
+Overall 
+
+![](images/vis/visualisationSequence.png)


Should there be a <<class>> above Categorizer and Visualizer?

ChoonSiang · 2023-11-01T04:37:51Z

+
+#### Diagrams
+Given below is the class diagram showing the class structure of the add income/expense mechanism:
+![](images/CashflowClassDiagram.png)


Perhaps you will want to refer to the PlantUML docs to remove the C and E circles to comply with our standard of UML diagrams.

ChoonSiang · 2023-11-01T04:38:53Z

+According to the cashflow type (Income/Expense) arugment passed in, the Categorizer sorts the 
+specified cashflow entry according to type using a Hashmap which is returned and used by the Visualizer
+
+Visualizer's Role: 


Nice use charts for user to visualize their income/expenses.

ChoonSiang · 2023-11-01T04:41:36Z

+![](images/CashflowClassDiagram.png)
+
+Given below is the sequence diagram showing the add income/expense mechanism:
+![](images/AddCashflowSequence.png)


Activation Bar is missing in this diagram. May want to refer to the Sequence Diagram examples in the CS2113 Website.

ChoonSiang · 2023-11-01T04:48:15Z

+
+This feature has 5 functions, `set`, `update`, `delete`, `reset`, and `view`.
+
+![](images/Budget.png)


Nice use of reference frames for neater view of the diagram.

ChoonSiang

Add on to previous comment

ChoonSiang · 2023-11-01T04:54:47Z

+

 ## Product scope
 ### Target user profile


May want to add more User Stories to accurately reflect your actual feature implementation in each version.

Branch update dg

Branch ug quickfix

Fix bug in UGDG

Changes dg

Fix broken link

Add about us

Changes dg

Fix bug in UG

Branch dg quick fix

…master Update

Fix bug

Final Final FInal Final changes

Add project portfolio page

Update aboutus page

Remove the non-existing John Doe from AboutUs.md

Brian030601 pushed a commit to Brian030601/tp that referenced this pull request Oct 18, 2023

Merge pull request nus-cs2113-AY2324S1#17 from junhyeong0411/master

93ae4b1

Make format of flashcard Directory

maanyos reviewed Oct 22, 2023

View reviewed changes

NeoMinWei and others added 12 commits October 31, 2023 22:50

Change method name for clarity

b5ba1de

Update DG with recur feature

e7355e4

Add recur class and sequence diagrams

bedd062

Merge branch 'master' of https://github.com/AY2324S1-CS2113-T18-2/tp

9442c88

Merge branch 'master' into Branch-UpdateDG

7edf450

Solve suggestions of Minwei

4257110

Update DG content page

0aa5efa

Update UG explaining the meaning of cashflow

8ef4691

Solve minor errors.

425cf72

Merge pull request #134 from hshiah/hshiah

1642706

Delete Reminder and display reminder

Add some fix to DG and UG

6ac95c6

Merge branch 'master' into VisDG

59beb3d

woodenclock reviewed Nov 1, 2023

View reviewed changes

danielpappa reviewed Nov 1, 2023

View reviewed changes

ChoonSiang reviewed Nov 1, 2023

View reviewed changes

NeoMinWei and others added 12 commits November 1, 2023 20:08

Update DG

f2077fc

Fix typo to be in camelCase

53cc8ca

Merge branch 'master' of https://github.com/AY2324S1-CS2113-T18-2/tp

be61c28

Merge branch 'master' into Branch-UpdateDG

8e6bc95

Merge pull request #135 from NeoMinWei/Branch-UpdateDG

35f10a3

Branch update dg

Merge branch 'master' of https://github.com/AY2324S1-CS2113-T18-2/tp

660f302

FIx formatting error in UG

c9b08fc

Merge pull request #140 from NeoMinWei/Branch-UGquickfix

3813917

Branch ug quickfix

Merge branch 'master' of https://github.com/AY2324S1-CS2113-T18-2/tp

5d2940d

Finish all user stories for v2.0

d90ccc5

Add help command

2ecb578

Update EXPECTED.TXT for UI testing

3f80f21

ryan1604 and others added 18 commits November 13, 2023 17:56

Merge pull request #216 from ryan1604/branch-bugfix

9bd2f73

Fix bug in UGDG

Merge pull request #217 from wwweert123/changesDG

d4d6562

Changes dg

Fix broken link

875b0ab

Merge pull request #218 from wwweert123/changesDG

1f3e3e5

Fix broken link

Add about us

1852642

Merge pull request #219 from wwweert123/changesDG

fbeda55

Add about us

Merge branch 'master' of https://github.com/AY2324S1-CS2113-T18-2/tp

9b7a4b7

Fix DG and UG

a7bbd83

Fix

6f57dc6

fix typo

1e59dff

Merge pull request #220 from wwweert123/changesDG

4a5e4e4

Changes dg

Fix bug in UG

6bff288

Merge pull request #221 from ryan1604/branch-bugfix

2a15144

Fix bug in UG

Add fix

3a324b7

Merge branch 'master' of https://github.com/AY2324S1-CS2113-T18-2/tp

99b8c54

Merge pull request #222 from wwweert123/changesDG

1c915c4

Update add cashflow explanations

e4f7d1e

Merge pull request #223 from NeoMinWei/Branch-DGQuickFix

0b7dbb5

Branch dg quick fix

Brian030601 pushed a commit to Brian030601/tp that referenced this pull request Nov 13, 2023

Merge pull request nus-cs2113-AY2324S1#17 from AY2324S1-CS2113-F11-3/…

bdcd066

…master Update

ryan1604 and others added 11 commits November 14, 2023 00:31

Fix bug

9c15ad8

Fix bug

bdb553a

Merge pull request #224 from ryan1604/branch-bugfix

30ef4ff

Fix bug

Final Final FInal Final changes

c4a964a

Merge pull request #225 from wwweert123/changesDG

2de04b7

Final Final FInal Final changes

Add project portfolio page

80da84b

Merge pull request #226 from YFshadaow/yfshadaow-project-portfolio-page

e97054c

Add project portfolio page

Update aboutus page

be44706

Merge pull request #227 from YFshadaow/yfshadaow-project-portfolio-page

708646a

Update aboutus page

Remove the non-existing John Doe from AboutUs.md

878b5ac

Merge pull request #228 from YFshadaow/yfshadaow-project-portfolio-page

3b2bd80

Remove the non-existing John Doe from AboutUs.md

		@@ -0,0 +1,5 @@
		package seedu.financialplanner.commands;

		public abstract class AbstractCommand {

		public static final CashflowList INSTANCE = new CashflowList();
		public final ArrayList<Cashflow> list = new ArrayList<>();


		{list here sources of all reused/adapted ideas, code, documentation, and third-party libraries -- include links to the original source as well}

		## Design & implementation


		{Describe the design and implementation of the product. Use UML diagrams and short code snippets where applicable.}

		### Storage Component


		This feature has 5 functions, `set`, `update`, `delete`, `reset`, and `view`.

		![](images/Budget.png)


		Overall

		![](images/vis/visualisationSequence.png)


		Overall

		![](images/vis/visualisationSequence.png)

Uh oh!

Conversation

ryan1604 commented Oct 5, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

maanyos left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

woodenclock left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

danielpappa left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ChoonSiang left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ChoonSiang left a comment

Choose a reason for hiding this comment

Uh oh!

ryan1604 commented Oct 5, 2023 •

edited

Loading