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

Diagram mismatch #12

Open
GenFusion122 opened this issue Nov 11, 2022 · 1 comment
Open

Diagram mismatch #12

GenFusion122 opened this issue Nov 11, 2022 · 1 comment

Comments

@GenFusion122
Copy link
Owner

GenFusion122 commented Nov 11, 2022

image.png

This sequence diagram has a interestList:Set however, there is no mention of said interestList within the Model class diagram on page 9. This causes some confusion in the implementation, does the interestList reside in the Student Object or a collated list in the model? Since the Class Diagram implies that a user contains multiple Interest objects with a composition relationship, directly contradicts the sequence diagram.

@nus-pe-script
Copy link

nus-pe-script commented Nov 15, 2022

Team's Response

Over here, we interpreted "Model" to be the domain logic layer, so we drew classes that were instantiated when we are performing the domain logic, which includes a Set that was created. interestList is the Set of Interest that is created to be passed as an argument to be added into a Student.

Since we did create a Set object containing the Interests, it is fine to draw it. While it may be a bit debatable to consider it as the Model layer, it is not a major error and it can vary whether it constitutes to be in the domain logic of the application.

It was also clearly explained how interestList is implemented. The attached figure is the writeup of the addInt feature that comes directly before the sequence diagram mentioned in this issue.

It is mentioned that

AddInterestCommandParser#parse parses the command arguments and returns a set of Interests.

Screenshot 2022-11-14 at 3.12.35 PM.png

While we could make it clearer by addressing the ambiguity of what constitutes the "Model" layer, this is not very major and we have revised it to be of Low severity.

Items for the Tester to Verify

❓ Issue severity

Team chose [severity.Low]
Originally [severity.Medium]

  • I disagree

Reason for disagreement: [replace this with your explanation]


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