search

johnnythesnake12 / pe

0 stars 0 forks source link

Inconsistent naming of the object. Hard to tell if person and employee have the same meaning #17

Open johnnythesnake12 opened 10 months ago

johnnythesnake12 commented 10 months ago

In the documentation, sometimes person is used such as error message being "The person index provided is invalid"

Other times, employee is used in why it happens, such as "The index specified does not refer to any employee"

Could consider adding a disclaimer that in the code, it is stored as a Person object, but Person and Employee has the same meaning

nus-pe-script commented 10 months ago

Team's Response

Same issue of employee/person naming.

The 'Original' Bug

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

Reference to person instead of employee

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 UG usually refers to the data in the app as belonging to employees. However, the UG occasionally mentions person. Though it is generally understandable that person = employee, this discrepancy hinders the overall direction of the app as a purpose-built system for employee tracking. This detracts from its appeal to the target audience, who wish to track their employees.

image.png (the example above is taken from page 17 of the UG)

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

Their Response to the 'Original' Bug

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

Our application only involves two main classes: Leaves and Employees, therefore the use of "person" is not ambiguous, as we are referring to the latter. We can still match this error message to the table. If a user reads the entire table, one would still be able to glean that there is no distinction between employee and person.

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`] - [x] I disagree **Reason for disagreement:** A HR Manager is considered a Person as well, testers will then be potentially confused whether the developers are referring to employees or HR Manager when developer chooses to use the word "Person"
## :question: Issue severity Team chose [`severity.VeryLow`] Originally [`severity.Medium`] - [x] I disagree **Reason for disagreement:** This issue should at least be low, but not very low as there might be confusion amongst testers or readers of their UG.