[CS2113-T12-2] BookKeeper #47

dylankhoo · 2025-03-02T05:24:07Z

BookKeeper helps librarians efficiently update book inventories, track borrowed and returned books with borrower details, and streamline operations like categorization and tracking, ensuring a smooth and organized library experience.

wenxin-c

Overall well done in the tp so far. Good use of abstraction and OOP. Minor details especially in the sequence diagram for your consideration. Keep it up.

wenxin-c · 2025-03-29T04:59:09Z

docs/images/addLoan.png

There shouldn't be a break for InputHandler activation bar when it is calling InputParser method. You might consider summarise the initial add loan xxx or make it multiple lines so that the diagram will be larger.

wenxin-c · 2025-03-29T05:01:12Z

docs/images/removeBook.png

Is there supposed to be a self-invocation at the start of the activation bar for InputHandler?

wenxin-c · 2025-03-29T05:13:49Z

src/main/java/bookkeeper/InputHandler.java

+                    case "add-book":
+                        addBook(commandArgs);
+                        break;
+                    case "view-inventory":


You could consider converting the commands into named constants so that it could be easier to use in other places and to maintain in the future, similar to magic numbers or literals that you might use frequently/change in the future.

wenxin-c · 2025-03-29T05:19:45Z

src/main/java/bookkeeper/InputParser.java

+     */
+    public static String[] extractAddBookArgs(String input) throws IncorrectFormatException {
+        // Split the input into required fields and optional note
+        String[] splitInput = input.trim().split("( a/)|( cat/)|( cond/)|( loc/)", 5);


You might consider making the array name sound more noun and plural, e.g. inputParts, splittedInputs etc.

wenxin-c · 2025-03-29T05:24:05Z

src/test/java/bookkeeper/InputParserTest.java

+//    @Test
+//    void extractCommandArgs_missingArguments_exceptionThrown() {
+//        IncorrectFormatException exception = assertThrows(IncorrectFormatException.class, ()
+//                        -> InputParser.extractCommandArgs("delete-loan "));
+//        assertEquals("Invalid command format.\nExpected: COMMAND [ARGUMENTS]", exception.getMessage());
+//    }


Remember to remove commented code.

Jasonlobo21

Overall, good effort in the DG heading towards the right direction

Jasonlobo21 · 2025-04-02T07:23:48Z

docs/DeveloperGuide.md

+
+The following UML sequence diagram shows how the `add-loan BOOK_TITLE n/BORROWER_NAME d/RETURN_DATE p/PHONE_NUMBER e/EMAIL` command is handled.
+
+![addLoan.png](images/addLoan.png)


Overall, i think the SD are great across all the ones in the developer guide. However, in all the SD, the header are mislabeled, which is missing the : for eg :InputHandler

Jasonlobo21 · 2025-04-02T08:04:03Z

docs/DeveloperGuide.md

+The following UML sequence diagram shows how the `add-loan BOOK_TITLE n/BORROWER_NAME d/RETURN_DATE p/PHONE_NUMBER e/EMAIL` command is handled.
+
+![addLoan.png](images/addLoan.png)
+


Maybe instead of labling "add-loan BOOK_TITLE n/BORROWER_NAME d/RETURN_DATE p/PHONE_NUMBER e/EMAIL" can just write in simple singlish eg add loan with book title, borrower name. Because when using developer guide may not have the code to understand the functions straight away.

Jasonlobo21 · 2025-04-02T08:05:06Z

docs/DeveloperGuide.md

+`InputHandler` coordinates with `BookList`, `LoanList` and `Formatter` classes to implement the feature.
+
+The following UML sequence diagram shows how the `remove-book TITLE` command is handled.
+![removeBook.png](images/removeBook.png)


For this SD, missing dotted line back to user even if its just a success message to alert the calling of function is done

Jasonlobo21 · 2025-04-02T08:07:06Z

docs/DeveloperGuide.md

+if you have not organized the code into clearly divided components (no penalty if you didn't), you can use a single
+class diagram (if it is not too complicated) or use several class diagrams each describing a different area of the
+system.}
+


could potentially include architecturediagram and OD for the InputParser, BookList, Formatter, and Storage`.

Jasonlobo21 · 2025-04-02T08:08:20Z

docs/DeveloperGuide.md

+   `InputHandler` calls `Storage.saveInventory(...)` to save the updated inventory.
+
+7. Success message is displayed:
+   `Formatter` is used to print a message indicating successful removal.


Overall, I think even the explanation of implementation is very good. Keep up the good work!

Jasonlobo21 · 2025-04-02T08:10:47Z

docs/DeveloperGuide.md

+The method `saveInventory(bookList)` is invoked by `InputHandler` after any method call that makes changes to the current inventory.
+
+The following UML sequence diagram shows the relevant behaviour:
+![saveInventory.png](images/saveInventory.png)


For the first alt bar which call mkdirs, Should include a small activation bar there as well as a dotted arrow to return to files's activation bar

Jasonlobo21 · 2025-04-02T08:12:04Z

docs/DeveloperGuide.md

+The method `loadInventory()` is called once by `InputHandler` at the start of the program.
+
+The following UML sequence diagram shows the relevant behaviour:
+![loadInventory.png](images/loadInventory.png)


I think could include activation bars for the missing one in the SD and dotted arrow returning once job is done

Yshinprograms · 2025-04-03T03:29:32Z

docs/DeveloperGuide.md

+```
+
+The following is the class diagram of the application:
+![](images/classDiagram.png)


Perhaps the class diagram could be made more concise as the attributes and methods look quite lengthy with all of the various types present? Maybe the relevance of the attributes and methods to the explanation within implementation later on could be improved upon?

Yshinprograms · 2025-04-03T03:32:11Z

docs/DeveloperGuide.md

+
+The following UML sequence diagram shows how the `add-book add-book BOOK_TITLE a/AUTHOR cat/CATEGORY cond/CONDITION loc/LOCATION [note/NOTE]` command is handled.
+
+![addBook.png](images/addBook.png)


The entities in the addBook sequence diagram looks like they are missing colons

Yshinprograms · 2025-04-03T03:35:29Z

docs/DeveloperGuide.md

+
+The following UML sequence diagram shows how the `add-loan BOOK_TITLE n/BORROWER_NAME d/RETURN_DATE p/PHONE_NUMBER e/EMAIL` command is handled.
+
+![addLoan.png](images/addLoan.png)


I like how detailed the addLoan SD is! However, the number of frames and text in the method calls make the SD a bit hard to follow. Maybe some parts of the SD could be abstracted out?

Yshinprograms · 2025-04-03T03:44:06Z

docs/DeveloperGuide.md

+
+The following UML sequence diagram shows the relevant behaviour:
+
+![loadInventory.png](images/loadInventory.png)


The Storage entity doesn't seem to return at the end of its activation bar although the explanation notes that "Finally, the populated bookList is returned." which gives the idea that the returning of bookList should be terminating the activation bar.

Yshinprograms · 2025-04-03T03:49:28Z

docs/DeveloperGuide.md

+
+The following UML sequence diagram shows the behaviour of `delete-loans TITLE n/BORROWER_NAME`:
+
+![delete_loan.png](images/deleteLoan.png)


There looks to be an additional dotted line above the "dotted" commands uml note. Maybe this was left in on accident?

fix feedback #139

Xavierleejrui

DG review

Xavierleejrui · 2025-04-03T05:21:03Z

docs/DeveloperGuide.md

+      └── Formatter.java # Formats and displays output
+```
+
+The following is the class diagram of the application:


I believe that the class diagram is quite comprehensive, although it may be too comprehensive as it could be improved to give a better understanding of the overarching high level code.

Xavierleejrui · 2025-04-03T05:25:03Z

docs/DeveloperGuide.md

+The following is the class diagram of the application:
+![](images/classDiagram.png)
+
+## Implementation


Overall interesting implementation, however would it be better if you add in some design considerations to explain why you decided to go ahead with the different classes

Xavierleejrui · 2025-04-03T05:31:20Z

docs/DeveloperGuide.md

+![addBook.png](images/addBook.png)
+
+1. User issues command:
+   The user inputs the command in the CLI with the required arguments, e.g., `add-book The Great Gatsby a/F. Scott Fitzgerald cat/Fiction cond/Good loc/Shelf B1 note/important`.


Would this be too many inputs for the user to keep track of, especially with the a/ , cond/ terms at the start before every line. Would it be better to just parse through based on the inputs

Xavierleejrui · 2025-04-03T05:39:32Z

docs/DeveloperGuide.md

+6. Changes are saved to persistent storage:
+   `InputHandler` calls `Storage.saveLoans(...)` and `Storage.saveInventory(...)` to save the updated book details.
+
+7. Success message is displayed:


clear workflow good job

DG fixes

…into shengbin-DG-week11-fixes

DG - Fix Save Inventory Activation Bar

Add expected outcomes

…ixDG3

louistaii · 2025-04-03T12:09:50Z

docs/DeveloperGuide.md

+
+The following UML sequence diagram shows how the `add-book add-book BOOK_TITLE a/AUTHOR cat/CATEGORY cond/CONDITION loc/LOCATION [note/NOTE]` command is handled.
+
+![addBook.png](images/addBook.png)


Would it be better to add the else statements as well into your alt blocks?

louistaii · 2025-04-03T12:24:17Z

docs/DeveloperGuide.md

+
+The following UML sequence diagram shows the relevant behaviour:
+
+![loadInventory.png](images/loadInventory.png)


The other sequence diagrams seem to have a return arrow from the Formatter after print functions but this one does now. Would be good to standardize.

…into shengbin-DG-week11-fixes

the notes are weirdly positioned but I couldn't find a way to make it arranged nicer...

DG - Fix Load Inventory Sequence Diagram and Explanation

the notes are weirdly positioned but I couldn't find a way to make it arranged nicer...

…ixDG3 # Conflicts: # docs/diagrams/classDiagram.puml # docs/images/classDiagram.png

Fix InputParser to handle names with "s/o"

…into shengbin-PED-fixes

Fix update-title to use case-sensitive comparison

Update developer guide

Search-title Sequence Diagram in DG

Update missing info

…code

DG Sequence Diagram Minor Edits

Update shengbin PPP

Update dylan-PPP

Update dylan-PPP for page limit

Update dylan-PPP for page length constraints

Edit PPP shengbin-101.md

Edit PPP shengbin-101.md, to fit to 2 pages

dylankhoo changed the title ~~[CS2113T-T12-2] BookKeeper~~ [CS2113-T12-2] BookKeeper Mar 2, 2025

wenxin-c reviewed Mar 29, 2025

View reviewed changes

Jasonlobo21 reviewed Apr 2, 2025

View reviewed changes

Yshinprograms reviewed Apr 3, 2025

View reviewed changes

Phua-Lock-Hian and others added 2 commits April 3, 2025 12:55

Merge pull request #146 from Phua-Lock-Hian/lockhian-fixDGissues

0122b0e

fix feedback #139

fix feedback #143

7af9949

Xavierleejrui reviewed Apr 3, 2025

View reviewed changes

Phua-Lock-Hian and others added 13 commits April 3, 2025 13:49

fix feedback #141

f7676b5

Merge pull request #147 from Phua-Lock-Hian/lockhian-fixDG2

635e6e3

DG fixes

Rename addloan.puml to addLoan.puml

d668e5d

Merge branch 'master' of https://github.com/AY2425S2-CS2113-T12-2/tp …

960cb07

…into shengbin-DG-week11-fixes

fix naming for FileWriter object and saveInventory Sequence Diagram

9fb6456

Fix instantiation and destroy File and FileWriter

e40619c

Merge pull request #148 from ShengBin-101/shengbin-DG-week11-fixes

66dd6b5

DG - Fix Save Inventory Activation Bar

Add expected outcomes

2c72f16

Merge pull request #149 from dylankhoo/update-user-guide

9a9496f

Add expected outcomes

Update break 1

d2b67c2

fix feedback #140

506d199

fix feedback #140

081f034

Merge remote-tracking branch 'origin/lockhian-fixDG3' into lockhian-f…

9257b6a

…ixDG3

louistaii reviewed Apr 3, 2025

View reviewed changes

ShengBin-101 and others added 9 commits April 3, 2025 20:34

Fix loadInventory sequence diagram

deb8248

Merge branch 'master' of https://github.com/AY2425S2-CS2113-T12-2/tp …

5e8333e

…into shengbin-DG-week11-fixes

Fix loadInventory sequence diagram and explanation

b7e0d80

fix feedback #138

aa3a89e

the notes are weirdly positioned but I couldn't find a way to make it arranged nicer...

Merge pull request #150 from ShengBin-101/shengbin-DG-week11-fixes

ee80622

DG - Fix Load Inventory Sequence Diagram and Explanation

fix feedback #138

8d47b26

the notes are weirdly positioned but I couldn't find a way to make it arranged nicer...

edit via MS paint

fef2b94

Merge remote-tracking branch 'origin/lockhian-fixDG3' into lockhian-f…

104bf7e

…ixDG3 # Conflicts: # docs/diagrams/classDiagram.puml # docs/images/classDiagram.png

put back attributes for Storage

f4c83e2

ShengBin-101 and others added 30 commits April 8, 2025 02:03

Fix Merge Conflicts

1c69735

Merge pull request #243 from ShengBin-101/shengbin-PED-fixes

4a15287

Fix InputParser to handle names with "s/o"

Update DG to use searchBook

e154ccb

Fix update-title to use case-sensitive comparison

fa1e65e

Merge branch 'master' of https://github.com/AY2425S2-CS2113-T12-2/tp …

046c98c

…into shengbin-PED-fixes

Rename searchBook() in InputHandler to searchTitle()

65b3dfd

Update all sequence diagrams to use searchBook

1f2533c

Add sequence diagram for search-title

71f2876

Update DG to include loadLoans and saveLoans

d7715a6

Merge pull request #245 from ShengBin-101/shengbin-PED-fixes

70bbe75

Fix update-title to use case-sensitive comparison

Merge branch 'master' into update-developer-guide

a558fb7

Merge pull request #246 from dylankhoo/update-developer-guide

3e80b35

Update developer guide

Search-title Sequence Diagram in DG

f9a5efe

Merge pull request #247 from ShengBin-101/shengbin-PED-fixes

77a6c10

Search-title Sequence Diagram in DG

Update missing info

ec6e09a

Merge pull request #248 from dylankhoo/update-user-guide

267cc26

Update missing info

add commandArgs into the method calls as arguments to reflect actual …

7559a6e

…code

Merge pull request #249 from Phua-Lock-Hian/lockhian-finalDGchecks

1f6bcaf

DG Sequence Diagram Minor Edits

Update shengbin PPP

0d14377

Merge pull request #250 from ShengBin-101/shengbin-PPP

9b24208

Update shengbin PPP

Update dylan-PPP

649232d

Merge pull request #251 from dylankhoo/dylan-PPP

24861f8

Update dylan-PPP

Update dylan-PPP for page limit

cc39095

Merge pull request #252 from dylankhoo/dylan-PPP

1238922

Update dylan-PPP for page limit

Update dylan-PPP for page length constraints

3563f72

Merge pull request #253 from dylankhoo/dylan-PPP

e6584a0

Update dylan-PPP for page length constraints

Edit PPP shengbin-101.md

83edb52

Merge pull request #254 from ShengBin-101/shengbin-PPP

3b832ef

Edit PPP shengbin-101.md

Edit PPP shengbin-101.md, to fit to 2 pages

514af59

Merge pull request #255 from ShengBin-101/shengbin-PPP

508e6e2

Edit PPP shengbin-101.md, to fit to 2 pages


		The following UML sequence diagram shows how the `add-loan BOOK_TITLE n/BORROWER_NAME d/RETURN_DATE p/PHONE_NUMBER e/EMAIL` command is handled.

		![addLoan.png](images/addLoan.png)


		The following UML sequence diagram shows how the `add-book add-book BOOK_TITLE a/AUTHOR cat/CATEGORY cond/CONDITION loc/LOCATION [note/NOTE]` command is handled.

		![addBook.png](images/addBook.png)


		The following UML sequence diagram shows the relevant behaviour:

		![loadInventory.png](images/loadInventory.png)


		The following UML sequence diagram shows the behaviour of `delete-loans TITLE n/BORROWER_NAME`:

		![delete_loan.png](images/deleteLoan.png)

[CS2113-T12-2] BookKeeper #47

Are you sure you want to change the base?

[CS2113-T12-2] BookKeeper #47

Uh oh!

Conversation

dylankhoo commented Mar 2, 2025

Uh oh!

wenxin-c 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!

Jasonlobo21 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!

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!

Xavierleejrui 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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants