Missing Implementation Details?

[gremmyz]

Currently,

there are only implementation details for the findBook and borrow/return feature. Perhaps details for other commands such as addPerson, editPerson, and deletePerson/deleteBook should be added as well. This will give developers a better understanding of your product.

I think it could be quite important for deletePerson and deleteBook, especially when the flag -f is used, to show how the Book objects get unassigned from Person objects.

[nus-pe-bot]

Team's Response

We feel that addPerson, editPerson and deletePerson is quite intuitive to understand, so we presented the implementation details of findBook and borrow/return as we feel that it is more important in order for the developers to understand and not be confused by other simple features.

Items for the Tester to Verify

:question: Issue response

Team chose [response.Rejected]

• [x] I disagree

Reason for disagreement: After looking through your source code, I don't believe that your sole reasoning for only presenting the implementation details of findBook and borrow/return (without UML diagram for the latter 2) is because that the rest of the commands are more intuitive to understand. Rather, I believe it is because the team did not put in enough effort to create UML diagrams for their DG documentation.

Reasoning for belief:

1. Based on their diagram for the findBook feature, it seems like there are missing sections (e.g. FindBookParser)

2. Based on the source code (LoC wise), it seems like findBook was one of the easier implementations to document.

findBook

vs. addBook

vs. addPerson

vs. deleteBook

vs. deletePerson

vs. editBook

vs. editPerson

While I partially agree that perhaps addPerson and editPerson would be more intuitive to understand, (personally, I think they should be added here anyway because they are core commands of your product), I strongly believe that deletePerson is important enough to be presented as well.

(I also found another minor bug in your DG as per the screenshot here, where AddCommand is used instead of AddPersonCommand or AddBookCommand, but I will not ask for penalty for this because I missed out on it during the PE.)

This screenshot shows the Logic component implementation for how deletePerson works. However, it does not indicate how calling the -f flag returns the books that the person has borrowed, as mentioned in the UG:

This is pretty important for developers to know, because if developers have a lack of understanding about how this works, it could lead to potentially functionally incorrect enhancements, such as deletePerson leads to all borrowed books being returned as well, especially so since your DG lacks the UML diagram for the Borrow/return feature.

---