woojiahao / pe

1 stars 0 forks source link

Unclear class diagram role #19

Open woojiahao opened 10 months ago

woojiahao commented 10 months ago

Unclear class diagram association role under "Model component" in DG. Such diagram can cause confusion in developers and make it harder to understand how the class is built. Additionally, the role of the "filtered and sorted" list does not point to a specific list and does not disambiguate between internalList and internalUnmodifiableList

Referring to the code, there's only two lists within the UniquePersonList class:


public class UniquePersonList implements Iterable<Person> {

    private final ObservableList<Person> internalList = FXCollections.observableArrayList();
    private final ObservableList<Person> internalUnmodifiableList =
            FXCollections.unmodifiableObservableList(internalList);

Screenshot 2023-11-17 at 17.20.59.png

nus-se-script commented 10 months ago

Team's Response

No details provided by team.

The 'Original' Bug

[The team marked this bug as a duplicate of the following bug]

Errors in the DG Model component diagram

Note from the teaching team: This bug was reported during the Part II (Evaluating Documents) stage of the PE. You may reject this bug if it is not related to the quality of documentation.


The Multiplicity and Roles in the diagram seem to be messed up. It is hard to tell which one is which.

image.png


[original: nus-cs2103-AY2324S1/pe-interim#5316] [original labels: type.DocumentationBug severity.VeryLow]

Their Response to the 'Original' Bug

[This is the team's response to the above 'original' bug]

Thanks for pointing this out! We agree that this may cause confusion to the reader.

Items for the Tester to Verify

:question: Issue duplicate status

Team chose to mark this issue as a duplicate of another issue (as explained in the Team's response above)

Reason for disagreement: [replace this with your explanation]


## :question: Issue severity Team chose [`severity.VeryLow`] Originally [`severity.Medium`] - [x] I disagree **Reason for disagreement:** I had previously rated this with a **Medium** severity, but after thinking about it, I do think it is a **Low** severity issue, not just a **VeryLow** severity issue. This is not simply a cosmetic issue with the diagram, but a notational issue. As mentioned in my original report: > Such diagram can cause confusion in developers and make it harder to understand how the class is built. Additionally, the role of the "filtered and sorted" list does not point to a specific list and does not disambiguate between internalList and internalUnmodifiableList If the association role poorly conveys the role of the class, then it only serves to confuse the developer when reading the DG. This confusion can cause misinterpretations of the intended behaviour and structure of the class. This can it harder to navigate the codebase as they are trying to align their understanding of the DG with the codebase. As such, it does affect the usage of the DG, and should not be a **VeryLow**