[CS2113-W12-3] WheresMyMoney #4

Hackin7 · 2024-09-18T05:54:13Z

WheresMyMoney helps frugal and tech-savvy university students track how much they are spending and what they are spending it on. The application can provide summaries and statistical insights on their spending habits, optimised for people who prefer a CLI

okkhoy

Please pay attention to coding standards.
Many non-trivial methods don't have header comments

okkhoy · 2024-10-26T15:15:21Z

src/main/java/wheresmymoney/Expense.java

+    public Float getPrice() {
+        return price;
+    }
+    public String getDescription() {
+        return description;
+    }
+    public String getCategory() {
+        return category;
+    }
+    public void setPrice(Float price) throws WheresMyMoneyException {
+        if (price == null) {
+            throw new WheresMyMoneyException("Expense's price shouldn't be null.");
+        }
+        this.price = price;
+        assert this.price != null : "Expense's price shouldn't be null.";
+    }
+    public void setDescription(String description) throws WheresMyMoneyException {


include some blank lines in between the methods for better readability

okkhoy · 2024-10-26T15:15:49Z

src/main/java/wheresmymoney/ExpenseList.java

+
+
+    /**
+     * Add an expense to the end of the list


doesn't follow our guidelines for header comments

okkhoy · 2024-10-26T15:16:29Z

src/main/java/wheresmymoney/ExpenseList.java

+                addExpense(Float.parseFloat(line[2]), line[1], line[0]);
+            }
+
+            // closing writer connection


It's a redundant comment and also looks incorrect.
Comment mentions writer; code references reader!

okkhoy · 2024-10-26T15:17:32Z

src/main/java/wheresmymoney/command/Command.java

+
+import java.util.HashMap;
+
+public abstract class Command {


this and its child classes can do better with some header comments

KSanjith

Almost there

KSanjith · 2024-10-30T05:08:17Z

docs/DeveloperGuide.md

+<u>Implementation Details</u>
+
+The following diagram is an inheritance diagram for Command and its children classes. 
+This has been heavily simplified and only shows the key commands.
+
+![CommandInheritance.png](diagrams%2Fimages%2FCommandInheritance.png)
+


Given how simplified it is and how few details it contains, is this diagram really necessary? Feel like you could either add more details or remove this diagram.

KSanjith · 2024-10-30T05:11:01Z

docs/DeveloperGuide.md

+
+The following diagram is a sequence diagram for execution of Command.
+
+![CommandExecutionSequence.png](diagrams%2Fimages%2FCommandExecutionSequence.png)
+


Is the execute method missing a return arrow and value?

KSanjith · 2024-10-30T05:12:43Z

docs/DeveloperGuide.md

+
+The following diagram is a UML class diagram for `Expense` and `ExpenseList`:
+
+![ExpenseAndExpenseList.png](diagrams%2Fimages%2FExpenseAndExpenseList.png "UML Class Diagram for Expense and ExpenseList")
+


Should multiplicity be 0..1 and not 0.1 ?

KSanjith · 2024-10-30T05:15:38Z

docs/DeveloperGuide.md

+
+Below is the UML class diagram for `RecurringExpense` and `RecurringExpenseList`:
+
+![RecurringExpenseAndRecurringExpenseList.png](diagrams%2Fimages%2FRecurringExpenseAndRecurringExpenseList.png "UML Class Diagram for RecurringExpense and RecurringExpenseList class")
+


Should multiplicity be 0..1 and not 0.1 ?

KSanjith · 2024-10-30T05:20:58Z

docs/DeveloperGuide.md

+
+![CommandExecutionSequence.png](diagrams%2Fimages%2FCommandExecutionSequence.png)
+


The return arrow from isExit() method is labelled as "boolean", which is a variable type. Should it instead be "exit :boolean" or "exit" or something similar, instead of just "boolean"

KSanjith · 2024-10-30T05:29:39Z

docs/DeveloperGuide.md

+
+![RecurringExpenseLoadFromCsvSequence.png](diagrams%2Fimages%2FRecurringExpenseLoadFromCsvSequence.png "UML Sequence Diagram for calling load command")
+


When instantiating new objects for File, FileReader and CSVReader, should the arrow point to the side of the object box, with an activation box coming right out of the object box to represent the constructor? And maybe you could perhaps not include the "new" keyword in the arrow? Can refer to https://nus-cs2113-ay2425s1.github.io/website/schedule/week10/topics.html#W10-1 for the proper formatting for object creation in sequence diagrams.

ayushi0803

overall, a commendable effort that needs a little more work and you'll be there! good luck team x

ayushi0803 · 2024-10-30T05:25:13Z

docs/DeveloperGuide.md

can you make the definitino of 'expense' classes more specific? Is expense an overarching umbrella over ExpenseList and ExpenseFilter? Can you make it more specfific with an appended suffix perhaps?

what does the command class do? can it be described further? maybe you can make categories of classes and define each class under the relevant category to ensure a seamless explanation

i cant really see the 1? can it be rewritten as something else? or can it be highlighted to make it more visible because it looks like its a dotted line

could you use colours for class diagrams to increase readability? perhaps the line spacing can be increased too?

why are these lines of code overlapping the vertical box on the left side of the diagram eg - [while line = csvReader.readNext)!= null] [for each RecurringExpense] etc

overall, a commendable effort that needs a little more work and you'll be there! good luck team x

Sukkaito

LGTM! I noted some possible changes that you guys might want to apply. Keep the good work! 👍

Sukkaito · 2024-10-30T05:30:45Z

docs/diagrams/images/ExpenseAndExpenseList.png

I don't think constructors should return any value, as constructors don't really return anything 🤔

Also, should this be a "0..1"? (2 dots)

Sukkaito · 2024-10-30T05:32:54Z

docs/diagrams/images/RecurringExpenseAndRecurringExpenseList.png

Same issue here, constructors having return value. Shouldn't this be the case?

Same issue here, should this be "0..1" instead?

Sukkaito · 2024-10-30T05:37:53Z

docs/diagrams/images/RecurringExpenseLoadFromCsvSequence.png

Should this be an opt block, because there is no "else" block in this case?

C2linaung · 2024-10-30T06:29:27Z

docs/DeveloperGuide.md

+2. `Expense`, `ExpenseList`, `ExpenseFilter` classes: Model expenses that commands can interact with.
+3. `Storage` class: Stores information between sessions.
+4. Logger and other utility classes: Provide extra functionalities for the software.
+### UI and Parser


the indentation is off. The formatting could be better

C2linaung · 2024-10-30T06:37:09Z

docs/DeveloperGuide.md

+
+The following diagram is a sequence diagram for execution of Command.
+
+![CommandExecutionSequence.png](diagrams%2Fimages%2FCommandExecutionSequence.png)


I think the diagram can be a bit confusing since i have to navigate both in the left and right direction. it might be better to should all the flows towards 1 direction

C2linaung · 2024-10-30T06:50:20Z

docs/DeveloperGuide.md

+
+![ExpenseAndExpenseList.png](diagrams%2Fimages%2FExpenseAndExpenseList.png "UML Class Diagram for Expense and ExpenseList")
+
+### Expense Filter


would it be better to have a sequence diagram to showcase each of the commands and how they work

C2linaung · 2024-10-30T06:51:36Z

docs/DeveloperGuide.md

+
+Commands interact with `UI` and `Parser` classes via `Main`, as illustrated in the following class diagram:
+
+![UiToCommand.png](diagrams%2Fimages%2FUiToCommand.png)


This has alot of details. Do you think we can make it more concise

AjayShanker-geek

Overall, your groups UG makes me worried for my team UG. Amazing work!

AjayShanker-geek · 2024-10-31T15:27:53Z

docs/UserGuide.md

+## Table of Contents
+- [Introduction](#introduction)
+- [Quick Start](#quick-start)
+- [Features](#features-)
+- [FAQ](#faq)
+- [Others](#others)
+- [Command Summary](#command-summary)


Great that you added Table of Content (TOC) to improve readability of the Developer Guide.

AjayShanker-geek · 2024-10-31T15:35:03Z

docs/diagrams/images/ArchitectureDiagram.png

Starting the Developer Guide with high-level overview diagram is good for me quickly understand architecture. The amazing part that is looks way better than plantuml.

AjayShanker-geek · 2024-10-31T15:40:23Z

docs/diagrams/images/UiToCommand.png

This is a minor issue, but it is difficult to see 1 at the head of the arrows.

AjayShanker-geek · 2024-10-31T15:50:14Z

docs/diagrams/images/RecurringExpenseLoadFromCsvSequence.png

Good effort on the sequence diagrams. Two minor issues: firstly will be the conditions overlapping with activation bar makes it hard to read. Secondly, should the function called be closer to the head of the arrow?

Update UG and Help Command with Set feature

Update Design Guide

Update UserGuide, HelpCommand and PPP

Update edit command

MVP of Visualization command

- Remove unused imports & wildcard imports - Visualizer can now work for time ranges up to three years

Add information about CategoryStorage

Correct link to category class UML diagram

Update

Final round of bug fixes

Add calculations section to DG

Update PPP details

Update documentation

Ignore the png in the commit before, that was the wrong png

Update DG

Fix typo in DG

into updatePPPandUGandDG # Conflicts: # docs/diagrams/images/UiToCommand.png # docs/diagrams/plantuml/UiToCommand.puml

Correct UMLs

Update PPP and fix wrong file name

Hackin7 force-pushed the master branch from b1b15f2 to edaac48 Compare October 18, 2024 07:10

okkhoy reviewed Oct 26, 2024

View reviewed changes

KSanjith reviewed Oct 30, 2024

View reviewed changes

ayushi0803 reviewed Oct 30, 2024

View reviewed changes

Sukkaito reviewed Oct 30, 2024

View reviewed changes

C2linaung reviewed Oct 30, 2024

View reviewed changes

AjayShanker-geek reviewed Oct 31, 2024

View reviewed changes

Progresst-8 and others added 23 commits November 4, 2024 21:56

Update UG FAQ

5829e9f

Update help command with set spending limits feature

1041f34

Merge pull request #104 from Progresst-8/UpdateCategoryDetails

b0c9f49

Update UG and Help Command with Set feature

Merge branch 'master' of https://github.com/AY2425S1-CS2113-W12-3/tp

0b86650

Merge branch 'master' into Edit-Command

f8927cd

Merge pull request #100 from KHsienKit/DesignGuide-RecurringExpense

54aeee7

Update Design Guide

Merge pull request #101 from shyaamald/branch-docs

3a0358b

Update UserGuide, HelpCommand and PPP

Merge branch 'master' of https://github.com/AY2425S1-CS2113-W12-3/tp

e713c43

Merge pull request #102 from KHsienKit/Edit-Command

aaae869

Update edit command

Merge branch 'master' of https://github.com/AY2425S1-CS2113-W12-3/tp

70c6072

Reformat method names in DG

19c8b47

Update class UML to exclude constructors

2eb72ef

Reformat method names

ec22f57

Add explanation for 2 add methods

f499559

Remove mention of month in command table for clarity

42763f0

Update date and time handling section

2948ca7

Add design consideration for dateutils

2355871

Add explanation to checkLimitOf to method table

a99fa39

Comment out attributes & methods that aren't planed to be discuss in DG

645c723

Add Visualization

3ad26aa

MVP of Visualization command

Make minor code changes

40c63eb

Update Visualizer

4f4e3eb

- Remove unused imports & wildcard imports - Visualizer can now work for time ranges up to three years

Fix CI

1c81953

Hackin7 and others added 30 commits November 11, 2024 05:17

Merge branch 'master' into Fix-Bugs

86f3a79

Fix CI

02205e1

Update DG with correct link to category package class UML.

471fca6

Add information about CategoryStorage

Update stats command

e6c13b7

Fix text-ui-test

55f1a00

Merge pull request #187 from Progresst-8/updatePPPandUGandDG

5818d20

Correct link to category class UML diagram

Fix text-ui-test ubuntu

359748f

Merge pull request #2 from AY2425S1-CS2113-W12-3/master

ce48603

Update

Merge pull request #183 from Hackin7/Fix-Bugs

2a91c1e

Final round of bug fixes

Add calculations section to DG

7d019c7

Merge branch 'master' into branch_testing

1a0331a

Add calculations to ToC

ff4beac

Merge pull request #188 from shyaamald/branch_testing

80655cb

Add calculations section to DG

Edit my name to my full name in AboutUs

ae1b731

Update PPP details

Merge pull request #189 from Progresst-8/updatePPPandUGandDG

4be2578

Update documentation

Fix typo in DG

7ceb488

Update classUML with correct parameters

701219e

Add png

5936273

Ignore the png in the commit before, that was the wrong png

Simplify sequence diagram

010f2a0

Update decorator capitalization indicating class level

c3cd9de

Update implementation description for CategoryFacade

7281452

Merge pull request #192 from Progresst-8/updatePPPandUGandDG

dd2fd06

Update DG

Merge pull request #191 from KHsienKit/V2.1

3cdcd94

Fix typo in DG

Correct UMLs

4b95853

Add isRecur

4d89985

Merge branch 'master' of https://github.com/AY2425S1-CS2113-W12-3/tp

21c893b

into updatePPPandUGandDG # Conflicts: # docs/diagrams/images/UiToCommand.png # docs/diagrams/plantuml/UiToCommand.puml

Update UMLs

7721936

Merge pull request #193 from Progresst-8/updatePPPandUGandDG

b1cf8b1

Correct UMLs

Update PPP and fix wrong file name

ac535b3

Merge pull request #194 from shyaamald/branch_testing

01d5ea4

Update PPP and fix wrong file name

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[CS2113-W12-3] WheresMyMoney #4

[CS2113-W12-3] WheresMyMoney #4

Hackin7 commented Sep 18, 2024

okkhoy left a comment

okkhoy Oct 26, 2024

okkhoy Oct 26, 2024

okkhoy Oct 26, 2024

okkhoy Oct 26, 2024

KSanjith left a comment

KSanjith Oct 30, 2024

KSanjith Oct 30, 2024

KSanjith Oct 30, 2024

KSanjith Oct 30, 2024

KSanjith Oct 30, 2024

KSanjith Oct 30, 2024

ayushi0803 left a comment

ayushi0803 Oct 30, 2024

ayushi0803 Oct 30, 2024

ayushi0803 Oct 30, 2024

ayushi0803 Oct 30, 2024

ayushi0803 Oct 30, 2024

ayushi0803 Oct 30, 2024

Sukkaito left a comment

Sukkaito Oct 30, 2024 •

edited

Loading

Sukkaito Oct 30, 2024

Sukkaito Oct 30, 2024

Sukkaito Oct 30, 2024

Sukkaito Oct 30, 2024

C2linaung Oct 30, 2024

C2linaung Oct 30, 2024

C2linaung Oct 30, 2024

C2linaung Oct 30, 2024

AjayShanker-geek left a comment

AjayShanker-geek Oct 31, 2024

AjayShanker-geek Oct 31, 2024

AjayShanker-geek Oct 31, 2024

AjayShanker-geek Oct 31, 2024


		The following diagram is a sequence diagram for execution of Command.

		![CommandExecutionSequence.png](diagrams%2Fimages%2FCommandExecutionSequence.png)


		The following diagram is a UML class diagram for `Expense` and `ExpenseList`:

		![ExpenseAndExpenseList.png](diagrams%2Fimages%2FExpenseAndExpenseList.png "UML Class Diagram for Expense and ExpenseList")


		Below is the UML class diagram for `RecurringExpense` and `RecurringExpenseList`:

		![RecurringExpenseAndRecurringExpenseList.png](diagrams%2Fimages%2FRecurringExpenseAndRecurringExpenseList.png "UML Class Diagram for RecurringExpense and RecurringExpenseList class")


		![RecurringExpenseLoadFromCsvSequence.png](diagrams%2Fimages%2FRecurringExpenseLoadFromCsvSequence.png "UML Sequence Diagram for calling load command")


		![ExpenseAndExpenseList.png](diagrams%2Fimages%2FExpenseAndExpenseList.png "UML Class Diagram for Expense and ExpenseList")

		### Expense Filter


		Commands interact with `UI` and `Parser` classes via `Main`, as illustrated in the following class diagram:

		![UiToCommand.png](diagrams%2Fimages%2FUiToCommand.png)

[CS2113-W12-3] WheresMyMoney #4

Are you sure you want to change the base?

[CS2113-W12-3] WheresMyMoney #4

Conversation

Hackin7 commented Sep 18, 2024

okkhoy 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

KSanjith 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

ayushi0803 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

Sukkaito left a comment

Choose a reason for hiding this comment

Sukkaito 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

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

AjayShanker-geek 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

Sukkaito Oct 30, 2024 •

edited

Loading