[CS2113-W10-4] uNivUSaver #6

DanLinhHuynh-Niwashi · 2024-09-18T10:48:25Z

uNivUSaver is a CLI-based software that helps students to develop a better habit of managing money.

joshuan98

Overall it seems like there are multiple functions that are quite long. You should consider refactoring these into separate files or functions to handle them instead.

joshuan98 · 2024-10-19T12:11:18Z

src/main/java/seedu/Main.java

@@ -0,0 +1,76 @@
+/*


I would recommend removing this file since it is entirely commented out.

src/main/java/seedu/command/AddExpenseCommand.java

joshuan98 · 2024-10-19T12:13:21Z

src/main/java/seedu/command/DeadAddTranctionCommand.java

@@ -0,0 +1,51 @@
+/*


Similarly, this entire file has been commented out. You can consider removing this,

src/main/java/seedu/main/Main.java

joshuan98 · 2024-10-19T12:15:32Z

src/main/java/seedu/main/Parser.java

+ */
+public class Parser {
+    /** Map that associates command words with their corresponding {@code Command} objects */
+    private static Logger logger = Logger.getLogger("Parser");


Should logger be declared final?

src/main/java/seedu/main/Parser.java

joshuan98 · 2024-10-19T12:18:28Z

src/test/java/seedu/command/HistoryCommandTest.java

+    private Transaction item5;
+    private Transaction item6;
+    @BeforeEach
+    public void setUp() throws Exception {


Good use of BeforeEach for setup

isaacsaw25 · 2024-10-30T04:23:31Z

docs/DeveloperGuide.md

+1. **commands: LinkedHashMap<String, Command>**
+  - Description: Associates command words (as keys) with their corresponding Command objects (as values).
+#### Class main methods
+1. **registerCommands(Command command): void**


The arguments should follow the UML style notation, namely
command : Command in the parameters

isaacsaw25 · 2024-10-30T04:24:01Z

docs/DeveloperGuide.md

+
+    ![register_command](./diagrams/parser/register-command-sequence.png)
+
+2. **parseCommand(String commandPart): Command**


Arguments here as well should follow the UML notation 😄

isaacsaw25 · 2024-10-30T04:28:08Z

docs/DeveloperGuide.md

+The `TransactionList` class is responsible for storing user transactions of different types. It also provides various
+operations that enable user to add, delete, search by (date/ category/ keywords).
+
+![TransactionList](./diagrams/TransactionList/transactionlist-class-diagram.png)


Great to see both Class diagrams and Sequence diagrams being used! 😸

isaacsaw25 · 2024-10-30T04:30:24Z

docs/DeveloperGuide.md

+
+2. **parseCommand(String commandPart): Command**
+  - **Parameters**: 
+    - `commandPart`: A string representing the command word entered by the user.


The formatting is a bit strange on the deployed website, where I believe you expected to place Returns: on a new line, but it prints on the same line as the explanation for the previous part 😰 . Perhaps you could try using a double space, which is kind of the equivalent of an escape character in Markdown. 👍

isaacsaw25 · 2024-10-30T04:33:27Z

docs/DeveloperGuide.md

+![Category](./diagrams/category/category-class-diagram.png)
+
+### TransactionList
+The `TransactionList` class is responsible for storing user transactions of different types. It also provides various


In the class diagram, I feel that you could exclude the get functions, as their usage and implementation are quite trivial to the architecture of the class. 😅

isaacsaw25 · 2024-10-30T04:41:22Z

docs/DeveloperGuide.md

+    - **Process**:
+        - Filters `transactions` to include only `Expense` objects with the specified `category`.
+        - Returns the filtered list of expenses.
+![SearchByCategory](./diagrams/TransactionList/transactionlist-Sequence-SearchByCategoty-diagram.png)


In this diagram, the 3rd self-invocation has a chain of 3 methods. Perhaps consider splitting the functions up into multiple invocations? 🐱

isaacsaw25

Great work overall, only minor flaws in the formatting to fix 😎

NigelYeoTW

Overall good progress, there seems to be some small issues with the diagrams but else lgtm, well done.

NigelYeoTW · 2024-10-30T06:20:00Z

docs/DeveloperGuide.md

+#### Class main methods
+1. **registerCommands(Command command): void**
+  - **Parameters**:
+    - `command`: The `Command` object to be registered.
+  - **Process**:
+    - Retrieves the `COMMAND_WORD` field from the `Command` object
+    - Adds the word and the command to the `commands` map.
+


Formatting error, also seen throughout the DG.

NigelYeoTW · 2024-10-30T06:27:58Z

docs/diagrams/parser/extract-arguments-sequence.png

There should not be a class at the end of the lifeline

NigelYeoTW · 2024-10-30T06:30:46Z

docs/diagrams/parser/register-command-sequence.png

self invocation should have a activation bar, comments can be placed inside notes

NigelYeoTW · 2024-10-30T06:32:27Z

docs/diagrams/parser/parse-command-sequence.png

inconsistency in parameters. some have class type, some do not

NigelYeoTW · 2024-10-30T06:34:34Z

docs/diagrams/category/category-class-diagram.png

additional Category class not used in diagram, Catergory2 could be named better

NigelYeoTW · 2024-10-30T06:35:41Z

docs/diagrams/addincomecommand/addincomecommand-class-diagram_001.png

sequence diagram class should have ':' regardless if there is a name for the class

NigelYeoTW · 2024-10-30T06:37:39Z

docs/diagrams/addincomecommand/addincomecommand-class-diagram.png

TransactionList has a missing dependency to Transaction class

not sure if Income should have a composition to TransactionList

NigelYeoTW · 2024-10-30T06:38:39Z

docs/diagrams/TransactionList/transactionlist-class-diagram.png

please follow UML notation for access modifier

NigelYeoTW · 2024-10-30T06:40:28Z

docs/diagrams/TransactionList/transactionlist-class-SearchKeyword-diagram.png

inside the [] should be the terminating condition

not too sure why there were ' in the diagrams

NigelYeoTW · 2024-10-30T06:46:38Z

docs/DeveloperGuide.md

+        - Aggregates and counts relevance for each match.
+        - Sorts the results by relevance and returns the list of matched transactions.
+![SearchByKeywords](./diagrams/TransactionList/transactionlist-class-SearchKeyword-diagram.png)
+5. **getExpensesByCategory(category : Category) : List<Transaction>**


There seems to be a formatting issue here seen throughout the DG as well

G13nd0n · 2024-11-03T11:48:49Z

docs/DeveloperGuide.md

@@ -1,29 +1,240 @@
 # Developer Guide


Good use of class and sequence diagram, would it be possible to add the object diagram as well?

G13nd0n · 2024-11-03T11:49:45Z

docs/DeveloperGuide.md

+
+### Class Main Methods
+
+1. **execute()**


Should it be execute() : List<String> instead?

G13nd0n · 2024-11-03T11:50:29Z

docs/DeveloperGuide.md

+
+  ![extract_arguments](./diagrams/parser/extract-arguments-sequence.png)
+
+4. **splitCommandRecursively(String argumentString, String[] keywords, Map<String, String> arguments, String prevKeyword): void**


The arguments should follow the UML notation, would it possible to amend it?

G13nd0n · 2024-11-03T11:53:43Z

docs/DeveloperGuide.md

@@ -1,29 +1,240 @@
 # Developer Guide


Overall, there is formatting issue especially the arguments for a few commands do not follow the UML notation

…into Parser-DeveloperGuide

Add follow-up question for expense clean up

…into SystemTest

Update author name

Update UserGuide.md add FAQ and notes

Update ppp

…UserGuide # Conflicts: # docs/UserGuide.md

Modify UserGuide

Update User guide

Update UserGuide.md change formatting

Update ppp

Change diagram and PPP

Update DG diagram

Update user guide for pdf print

Reformat UG

Reformat for printing

Fix link

update

joshuan98 reviewed Oct 19, 2024

View reviewed changes

isaacsaw25 reviewed Oct 30, 2024

View reviewed changes

NigelYeoTW reviewed Oct 30, 2024

View reviewed changes

G13nd0n reviewed Nov 3, 2024

View reviewed changes

docs/DeveloperGuide.md

### Class Main Methods

1. **execute()**

Copy link

G13nd0n Nov 3, 2024

Choose a reason for hiding this comment

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

Should it be execute() : List<String> instead?

G13nd0n reviewed Nov 3, 2024

View reviewed changes

LinhHuynhLeDan and others added 14 commits November 6, 2024 21:46

Merge branch 'master' of https://github.com/AY2425S1-CS2113-W10-4/tp …

4b02a33

…into Parser-DeveloperGuide

Add follow-up question for expense clean up

c46c020

Disable logging

70d7afb

Update test

94ce626

Fix contrustor bug

bcf525e

Fix checkstyle

1b5d54b

Merge pull request #126 from DanLinhHuynh-Niwashi/DeleteCategory

87b972f

Add follow-up question for expense clean up

Merge branch 'master' of https://github.com/AY2425S1-CS2113-W10-4/tp …

183a7b5

…into SystemTest

Update author name

1b1908d

Merge pull request #128 from DanLinhHuynh-Niwashi/DeleteCategory

28c4d3f

Update author name

Change command guide

3dfc269

Update test

3fa3d84

Update test

95a22f7

Update test

cff3e81

Oshen27 and others added 30 commits November 12, 2024 11:12

Update UG with example output

5421a9e

Merge pull request #229 from DanLinhHuynh-Niwashi/Documentation

4b4f405

Update UserGuide.md add FAQ and notes

Merge pull request #230 from YukeeHong/update_ppp

bd53e6e

Update ppp

update ppp

02b2f9a

Merge commit 'bd53e6efede1af45056f7cb480cf8cc5b339c7b2' into Oshen27-…

9b81df3

…UserGuide # Conflicts: # docs/UserGuide.md

Modify UserGuide

98a6832

Merge pull request #232 from Gao327/Branch-Gao327-final

a6681de

Modify UserGuide

Update PPP

c071154

Merge branch 'master' into Oshen27-UserGuide

d01b84c

Merge pull request #233 from Oshen27/Oshen27-UserGuide

02cd41d

Update User guide

Update UserGuide.md change formatting

73aa99f

Merge pull request #234 from DanLinhHuynh-Niwashi/Documentation

74bbdef

Update UserGuide.md change formatting

update ppp

d70fd44

Merge pull request #235 from YukeeHong/update_ppp

6278ace

Update ppp

Change diagram and PPP

61cba9f

Merge pull request #236 from DanLinhHuynh-Niwashi/Documentation

1ed42b9

Change diagram and PPP

Update DG diagram

6630e65

Update user guide for pdf print

fa3e5fe

Merge pull request #237 from Gao327/Branch-Gao327-final

b7e0ac8

Update DG diagram

Merge pull request #238 from DanLinhHuynh-Niwashi/Documentation

a0a3e85

Update user guide for pdf print

Reformat UG

6f0518e

Merge pull request #239 from DanLinhHuynh-Niwashi/Documentation

5859270

Reformat UG

Reformat for printing

3e3f4a3

Merge pull request #240 from DanLinhHuynh-Niwashi/Documentation

b94d6a2

Reformat for printing

Fix link

608cfb6

Merge pull request #241 from DanLinhHuynh-Niwashi/Documentation

5948250

Fix link

Fix link

5e38c8c

Merge pull request #242 from DanLinhHuynh-Niwashi/Documentation

1581b73

Fix link

update

e9a1e40

Merge pull request #243 from Gao327/master

d691992

update

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[CS2113-W10-4] uNivUSaver #6

[CS2113-W10-4] uNivUSaver #6

DanLinhHuynh-Niwashi commented Sep 18, 2024 •

edited

Loading

joshuan98 left a comment

joshuan98 Oct 19, 2024

joshuan98 Oct 19, 2024

joshuan98 Oct 19, 2024

joshuan98 Oct 19, 2024

isaacsaw25 Oct 30, 2024

isaacsaw25 Oct 30, 2024

isaacsaw25 Oct 30, 2024

isaacsaw25 Oct 30, 2024 •

edited

Loading

isaacsaw25 Oct 30, 2024

isaacsaw25 Oct 30, 2024

isaacsaw25 left a comment

NigelYeoTW left a comment

NigelYeoTW Oct 30, 2024

NigelYeoTW Oct 30, 2024

NigelYeoTW Oct 30, 2024

NigelYeoTW Oct 30, 2024

NigelYeoTW Oct 30, 2024

NigelYeoTW Oct 30, 2024

NigelYeoTW Oct 30, 2024

NigelYeoTW Oct 30, 2024

NigelYeoTW Oct 30, 2024

NigelYeoTW Oct 30, 2024

NigelYeoTW Oct 30, 2024

NigelYeoTW Oct 30, 2024

G13nd0n Nov 3, 2024

G13nd0n Nov 3, 2024

G13nd0n Nov 3, 2024

G13nd0n Nov 3, 2024


		![register_command](./diagrams/parser/register-command-sequence.png)

		2. parseCommand(String commandPart): Command


		![extract_arguments](./diagrams/parser/extract-arguments-sequence.png)

		4. splitCommandRecursively(String argumentString, String[] keywords, Map<String, String> arguments, String prevKeyword): void

[CS2113-W10-4] uNivUSaver #6

Are you sure you want to change the base?

[CS2113-W10-4] uNivUSaver #6

Conversation

DanLinhHuynh-Niwashi commented Sep 18, 2024 • edited Loading

joshuan98 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

isaacsaw25 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

isaacsaw25 left a comment

Choose a reason for hiding this comment

NigelYeoTW 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

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

DanLinhHuynh-Niwashi commented Sep 18, 2024 •

edited

Loading

isaacsaw25 Oct 30, 2024 •

edited

Loading