szelongq / pe

0 stars 0 forks source link

DG: Inaccuracy in class diagram #8

Open szelongq opened 2 years ago

szelongq commented 2 years ago

Description of Bug:
In the class diagram for the Model component, the notelist and uniquequestionlist to notes and questions respectively should be a composition relationship.

Screenshot 2021-11-12 at 5.38.54 PM.png

This is also inconsistent with the class diagram for Notes later on:

Screenshot 2021-11-12 at 5.39.30 PM.png

nus-pe-bot commented 2 years ago

Team's Response

Thank you for the report. We will be rejecting this report for the following reasons:

image.png

Based on the textbook, aggregations can be rather confusing to the user, and that was omitted in the model diagram as it we only wanted to show the big picture of how the noteslist was related to notes. We feel that this did not hinder the reader in any way whatsoever, as it is still understandable that the noteslist is linked to all notes, based on its multiplicity.

For the note diagram, we included it as the relationship is an important thing to know, whereas for our model diagram, we only want to show that smartNus is defined by the question list and note list, therefore, we omit all unnecessary details. Since the reader is still able to understand the association, it does not hinder them in any way and hence we reject this bug report.

Items for the Tester to Verify

:question: Issue response

Team chose [response.Rejected]

Reason for disagreement: Thank you for your response! I agree that unnecessary details should be omitted, and that aggregation relationships are one to be omitted as the textbook recommends. However, I think that the composition relationship in this diagram should be essential and should not be omitted.

It is not immediately understandable that there is a composition relationship just from the association arrow with multiplicity, otherwise I believe you would not have used the composition symbol in your second diagram. While you might have said that you only wish to show the relationship between smartNUS and the UniqueQuestionList and NoteList, since you have already included the Note and Question classes in the diagram as well, the absence of the composition relationship might cause a misunderstanding and omitting it does not really make it easier to digest.

Therefore, I still believe this is a bug.