search

L1uY1jun / pe

0 stars 0 forks source link

Developer Guide not updated for the application #8

Open L1uY1jun opened 1 year ago

L1uY1jun commented 1 year ago

Bug Description:

The application presented is an application designed towards managing module planning as specified under the user guides

image.png

However, the developer guide is not updated towards the logic of an application meant for managing module planning.

There are many instances of a mention about an addressbook, including under the Logic Component which is suggest that this application manages address of a person. However, that is not the case. The fact that addressbook is used in many instances can affect developers who are interested in finding out more about the technical aspects of this application.

Below are some examples.

Under Architecture -> How the architecture components interact with each other

image.png

Under Logic Component

image.png

Under Add Module Feature -> Alternatives considered

image.png

nus-se-script commented 1 year ago

Team's Response

Fixing #5445 to change everything to Plannit will automatically fix #2580 as well.

The 'Original' Bug

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

Still using AddressBook

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.

image.png

In the sequence diagram shown above, AddressBookParse should be changed to your product name.

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

Their Response to the 'Original' Bug

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

Our code retained the old class name from AB3 in our code base, hence the documentation is accurate. Take this for example: AddressBookParser

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 response

Team chose [response.Rejected]

Reason for disagreement: Justification for disagreement with response:

The logic of the application is supposed to be sound and coincide with the features provided by the application. Naming your parser AddressBookParser when your application does not handle address of any form does decreases the cohesiveness of the program. In your program, the details of a person only includes: name, email and phone number.

image.png

Although the standardization of the name is present, there is no coherence of the naming consideration with the features provided.

This means that for any developer that is not familiar with the AB3 brownfield project can be extremely confused with the logic given that your team wishes to retained the old class name from AB3 in the code base.

:question: Issue severity

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

Reason for disagreement: Justification for severity.Low:

This naming convention is not merely a cosmetic issue. It can hinder the reader, as explain above: The naming decreases the cohesiveness of the program. Which makes it confusing for developers unfamiliar with AB3 code base that is reading your code.

image.png