parth-io / pe

0 stars 0 forks source link

Overuse of class diagrams in DG #16

Open parth-io opened 1 year ago

parth-io commented 1 year ago

Class diagrams are overused and hamper readability.

For example, for the "Show Command", it is not really clear why the inclusion of this class diagram is useful to the reader. The class diagram is not directly referenced in the description and is just left for the reader to read through without any full understanding of its significance. Furthermore, it does not even seem significant or useful, as I can just read the code to understand what methods are implemented.

image.png

nus-se-script commented 1 year ago

Team's Response

This class diagram is to show the developer

  1. which classes are relevant
  2. which methods of the classes are at play

    Items for the Tester to Verify

    :question: Issue response

Team chose [response.Rejected]

Reason for disagreement: > to show which classes are relevant

If the consideration is to tell the future dev that a relevant class is TargetPerson, then this can be mentioned in one line. There is no need to include the class diagram to highlight that a class is relevant. So this explanation does not address why the class diagram itself is needed.

to show which methods of the classes are at play

This explanation potentially addresses why the class diagram is needed. But I don't think it is even helpful to show the methods that are at play. As mentioned in the original bug report, "the class diagram is not directly referenced in the description and is just left for the reader to read through without any full understanding of its significance". What this means is that the future dev can see that these methods exist in the class via the class diagram, but that's about it in terms of the usefulness of the class diagram. The future dev will not have an understanding of the design choices of the methods, or of why they exist, or of caveats in usage. Furthermore, the methods seem to be simple getters, and getters usually don't need an explanation or inclusion in the DG. So including these methods seem to be pointless, and hinder readability as the future dev will have to look at the diagram and understand for himself that the diagram is just listing the methods, and that there is no further relevance to the description later.

Implication --> at best, the class diagram is just quickly skimmed and skipped because it is not relevant to the description later. At worst, reading the DG is not painless because the user has to figure out if there is anything important in the diagram that the later description does not cover.

In either case, the quality of the DG is harmed.

This was specifically for ShowCommand, but similar arguments can be made for other class diagrams.