yan-soon / pe

0 stars 0 forks source link

Logic component not refactored accordingly #9

Open yan-soon opened 3 years ago

yan-soon commented 3 years ago

Diagrams in the Logic component still contain traces of AB3 implementation eg. Person instead of Contact and addressBook instead of StonksBook. Once again, if intended, can be quite misleading for readers who are also familiar with AB3.

image.png

nus-se-bot commented 3 years ago

Team's Response

Prof advised not to rename packages in Luminus announcements. Since we did not change the names of the AB3 related packages, hence the naming of the DG components remain the same.

In addition, there is no need to refactor your packages to match the naming of your product.

Items for the Tester to Verify

:question: Issue response

Team chose [response.Rejected]

Reason for disagreement: According to the image below, it is written that we can keep our package names the same as AB3, however we should change our class names to reflect what they represent. In the image flagged previously, both contact and person are used in the same diagram and it could lead to people thinking they are two different classes.

Furthermore, throughout the DG, there are multiple references to both Contact and Person, AddressBook and StonksBook, further perpatuating the belief that they are two different entities, but really they are referring to the same thing. I understand if the devs wanted to stick with the original Luminus announcement as stated and not rename anything package related. However, to avoid further misunderstanding, I feel that they could have avoided using Contact altogether and only used Person to describe the people stored in their StonksBook app, and all their related classes and method names. Same goes for AddressBook and StonksBook.

image.png

Example of StonksBook and AddressBook being referenced as the same item.

It says here on pg 25 of the DG that Sale objects are stored in AddressBook.

image.png

However, on pg 23, the Sale objects are now stored in StonksBook.

image.png