JonathanHoshi / pe

0 stars 0 forks source link

DG UML diagram and application still saves `addressbook` #7

Open JonathanHoshi opened 2 years ago

JonathanHoshi commented 2 years ago

Details

image.png

As shown above in the DG's architecture UML diagram, there is still a saveAddressBook(addressbook) method. Is this supposed to be saveCandidateList(candidateList) instead? If i recall correctly, TAlentAssistant stores an list of candidates. Even if the code is actually correct, addressbook does not seem to be the theme of what the application is trying to store. This is supported also as in some parts in the DG it shows the application storing Candidates (shown in the screenshot #2)

Screenshot

image.png

image.png

soc-pe-bot commented 2 years ago

Team's Response

Our team has prefaced at the top of the Design section - no refactoring has been done for AddressBook named classes and packages. The diagram follows the methods implemented in the actual codebase, hence there is no error in the naming.

image.png

Items for the Tester to Verify

:question: Issue response

Team chose [response.Rejected]

Reason for disagreement: ## Reason My main reason for rejecting the response is that AddressBook does not seem to fit the team's project as a naming convention. From what I notice, the team has already gone all their way to rename their classes, to the point that the AddressBook class contains a UniqueCandidateList and methods in AddressBook containing candidate methods (shown in screenshot #1). I feel that it would not be much work to change the class name to something that seems more fitting, for example CandidateList. I feel that stating that your team would like to keep AddressBook is not a solid reason to not wanting to name the class properly. Based on code quality in CS2103, the name of a entity is not just for differentiation but should clearly explain what the object is about (shown in screenshot #2). AddressBook does not clearly show that it is storing candidates at all.

The module have also shown that we are expected to rename the product as a necessary code enhancement (shown in screenshot #3).

Screenshot

1

image.png

2

image.png

3

image.png