delishad21 / pe

0 stars 0 forks source link

The DG only includes information for the implementation of 3 features #24

Open delishad21 opened 4 months ago

delishad21 commented 4 months ago

These are indeed the 3 main features of the application :Undo/Redo, Mailing, and Filter.

However, there is no mention of how the phone number copying feature is implemented. There is also no indication as to how the tagging and untagging feature is implemented.

soc-pe-bot commented 4 months ago

Team's Response

The developer guide does not need to include everything. It just needs to explain features that might be hard to understand or had special considerations.

Items for the Tester to Verify

:question: Issue response

Team chose [response.Rejected]

Reason for disagreement: It is true that the developer guide does not need to include everything. However, this statement only stands if the information already included is sufficient for assisting current and future developers. As mentioned in the course textbook, this can come in two ways: documentation for developer-as-user, and documentation for developer-as-maintainer

https://nus-cs2103-ay2324s2.github.io/website/schedule/week9/topics.html#type-of-developer-docs

The implementation section likely falls under the developer-as-maintainer section. Considering that not even the slightest bit of implementation details were given for the phone copying feature, I argue that the developer guide has failed in in its purpose to assist any developer in maintaining that portion of the code. Any developer who wishes to update or maintain the phone command would have to look through its code line by line just to grasp its logic.

Furthermore, the argument of

It just needs to explain features that might be hard to understand or had special considerations.

does not stand. The phone copying feature includes some design choices that are inconsistent with what is stated in the UG and would have benefited from explanations included in the DG. For example:

image.png

The user guide indicates that only one tag should be used for the filtering of phone numbers. However, the implementation in the parser for the phone command creates a predicate that uses an array of multiple "keywords". Why was this approach used? Wouldn't it make more sense to create a predicate to search for a single keyword instead?

image.png

Additionally, some design choices were not elaborated upon, such as the use of the model#updateFilteredPersonList method and the use of the Toolkit library.

image.png

As such, I believe this to be a crucial issue and should have been acknowledged by the team as a gap in their documentation.