StanleyNeoh / pe

0 stars 0 forks source link

Sequence diagram of components are not accurate #13

Open StanleyNeoh opened 1 year ago

StanleyNeoh commented 1 year ago

image.png

Under the section about How the architecture components interact with each other,

The reader can get the impression that the handling of incoming commands from the command box is done by an instance that inherit the Ui interface.

However, only UiManager inherits from the Ui interface but it is not UiManager class that is responsible of handling the command but the MainWindow class that is as shown here

image.png

nus-se-bot commented 1 year ago

Team's Response

This diagram is intended as a high level overview of how all the architectural components interact with each other.
Considering low level components like MainWindow would be a violation of SLAP, and would not relevant to the scope of this diagram, which deals with very high level components.
It is not necessary to consider precise details if it is not relevant to the context, and including it can potentially make the reader even more confused.

Screenshot 2023-04-17 at 14.34.50.png

Items for the Tester to Verify

:question: Issue response

Team chose [response.Rejected]

Reason for disagreement: The suggested change was to rename :Ui to :MainWindow as it is the MainWindow that handles the execution of the command.

As a developer trying to navigate the codebase, I find that it would be very helpful to know which class is responsible for the handling of user commands. Using :Ui can mislead the developer to look into the wrong files when trying to find which class handles user commands. Furthermore, opting to use :Ui instead of :MainWindow does not make the sequence diagram any simpler and only serves to mislead the developers reading the developer guide.

To address the violation of SLAP, I don't think that this is particularly relevant as the sequence diagram reflects how the application is implemented. As shown in the screenshot of the bug report, the application was implemented in a way such that it is MainWindow, not Ui that is responsible for handling user commands.

Hence, I do not agree that this issue should be rejected.