search

nus-cs2103-AY2122S1 / pe-dev-response

0 stars 0 forks source link

Suggestion: naming of classes should be changed. #5684

Open nus-se-bot opened 2 years ago

nus-se-bot commented 2 years ago

Note from the teaching team: This bug was reported during the Part II (Evaluating Documents) stage of the PE. You may reject this bug if it is not related to the quality of documentation.

The diagram still uses AddressBook although the application is now ForYourInterest. If the diagram is not outdated, you could consider refactoring AddressBook instances to your application name.

image.png

[original: nus-cs2103-AY2122S1/pe-interim#5666] [original labels: severity.Low type.DocumentationBug]

yongxiangng commented 2 years ago

Team's Response

image.png

This is consistent with our code, so not really a bug. As such, from now on, I shall address (hoho pun intended) the tester's concerns of "you could consider refactoring AddressBook instances to your application name"

Firstly downgraded this to veryLow, as it is merely a choice of name (hence cosmetic in nature)

Secondly, address book is a general term and not limited to AB3 (see the glossary image and google). Even though our application is not called addressbook, it does still use an addressbook in the implementation. As such there is no need to change the name, and the name actually fits the use case perfectly.

Duplicate status (if any):

--