Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sequence Diagram is Invalid in terms of the parse(), parseCommand() and execute() #15

Open
KhoonSun47 opened this issue Apr 19, 2024 · 1 comment

Comments

@KhoonSun47
Copy link
Owner

KhoonSun47 commented Apr 19, 2024

Steps to reproduce:

  • Currently, the execute(), parseCommand() and execute() in the sequence diagram under Logic Component does not have a parameter.
  • However, if you were to follow the sequence diagram that is under Edit Applicant Feature, there are parameters for parse(), parseCommand() and execute()
  • This results in inconsistency in the sequence diagram that may confuse the user.

Logic Component Sequence Diagram:
image.png

Edit Applicant Feature Sequence Diagram:
image.png

Suggestion:
I believe that you can make the sequence diagram more consistent by making sure there are parameters within the methods.

@nus-pe-script
Copy link

nus-pe-script commented Apr 22, 2024

Team's Response

The reported issue does not affect the quality of the documentation as the first diagram is intended for high level view for logic component and second diagram is for explaining in detail for editting applicants. Hence, it is rejected. Thanks!

Items for the Tester to Verify

❓ Issue response

Team chose [response.Rejected]

  • I disagree

Reason for disagreement: I disagree that this issue should be categorised as "response.Rejected" and it should be remained as “response.Accepted” instead. 


While your team response clarifies that the first diagram under "Logic Component" is designed to provide a high-level overview, it does not address the omission of parameters for the execute(), parseCommand(), and parse() methods. 


According to the textbook, if the parameters are irrelevant to the sequence diagram's intent, they should be omitted using ellipses ("...") rather than left blank. 


This inconsistency could cause confusion, especially since the second diagram, which is intended to detail these methods, includes parameters. 


Readers might question whether these are different methods or whether parameters should be included at all.

This clarification is crucial for ensuring accurate understanding and consistency throughout the Developer Guide.

• If method parameters don't matter to the purpose of the sequence diagram, you can.png


Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants