Silvernitro / pe

0 stars 0 forks source link

DG: "Reminder related classes" diagram #14

Open Silvernitro opened 3 years ago

Silvernitro commented 3 years ago

This diagram contains a bit too many low-level details such as the methods in ReminderManager that don't really tell the user anything without context. Listing of methods shouldn't be needed since the reader can simply look at the code, especially if there's no reference to those methods in the DG section abt that diagram.

Also, since the methods are also mentioned in the sequence diagram too, maybe they can be left out in this diagram to prevent repetition.

Screenshot 2020-11-13 at 5.41.13 PM.png

nus-se-bot commented 3 years ago

Team's Response

A lot of methods were already removed from the diagram, and the methods included in the diagram are all public methods that are exposed, and thus are necessary. Most of the methods are actually duplicates to show the different signatures for overloaded methods. We think it is important to show these methods as they are used later in sequence diagrams.

Items for the Tester to Verify

:question: Issue response

Team chose [response.Rejected]

Reason for disagreement: I believe that placing the class diagram before all the sequence diagrams that utilize the classes & methods does not add value to the reader, and only serves to clutter and confuse him/her.

  1. As mentioned by the team, since the methods shown will be used later in the sequence diagrams, the class diagram is somewhat redundant since readers can easily tell which class each method belongs to from the sequence diagram.
  2. But having said point 1, I do understand that looking at multiple sequence diagrams doesn't provide a quick overview of all methods in relation to the package's classes. But I do feel that placing the class diagram after all the sequence diagrams as a form of summary is more appropriate since readers would have already understood the context involved after reading the sequence diagrams. Perhaps a label such as "This class diagram provides an overview of all the previously discussed methods and classes in this section" can be used as the end.

My main gripe with this is the clutter combined with the lack of context. This class diagram is placed right after a short introduction of the "Reminders" section, and when I looked at the class diagram, I had no idea what I was supposed to understand from seeing it. Especially with the clutter, I had no idea which methods I was supposed to take notice of at this point since I had 0 clue how the feature worked. It was only after reading the rest of the sequence diagrams did I have some idea of how the classes and methods worked.

At the end of the day, this is indeed a minor inconvenience to the reader since they would be confused for some time until they read through the entire section. I thus believe that this should still be a doc bug with a very low severity.