Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Index used for commands are inconsistent #10

Open
ningtan11 opened this issue Nov 11, 2022 · 1 comment
Open

Index used for commands are inconsistent #10

ningtan11 opened this issue Nov 11, 2022 · 1 comment

Comments

@ningtan11
Copy link
Owner

ningtan11 commented Nov 11, 2022

Add-leave and delete command use ID/EMPLOYEE_ID instead of the straightforward INDEX that the commands to edit and view employee use. This may cause confusion to a user, and this tester is left wondering why there is a need for commands to use a different method of identifying the employee.

Might be a good idea to allow both ways of employee identification for both commands, so the user can use whichever is more convenient for them.

@ningtan11 ningtan11 changed the title Leave commands use parameters inconsistent with the rest of app Index used for commands are inconsistent Nov 11, 2022
@nus-se-script
Copy link

nus-se-script commented Nov 15, 2022

Team's Response

Same issue regarding the use of ID and index in different commands.

The 'Original' Bug

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

[Delete/Edit] Confusion between Index and Employee ID

In deleting an employee, we specify the Employee ID, however, in editing the employees, we are using the INDEX.

While it is specified in the UG, it could be potentially confusing to the user, who may be new to the system.

Suppose that I want to delete an employee, but I have used the INDEX instead of the Employee ID, this could affect my workflow substantially.

As such, it would be good to stick to either INDEX or Employee ID but not both.


[original: nus-cs2103-AY2223S1/pe-interim#2177] [original labels: type.FeatureFlaw severity.Low]

Their Response to the 'Original' Bug

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

Hi there! Thanks so much for the report.

The default usage is Employee ID. However there are two exceptions to this, the edit and view command. The view command uses index so that it can be used in conjunction with the find command (eg. Find a set of employees and then view employees) and we intended edit command to be used with the employee profile displayed as it is more likely that the user will want to view the current details of employee before making the edit.

We designed the commands with these intentions in mind to best facilitate the app usage. Therefore, we do not agree with your suggestion and will be rejecting this bug.

Once again, thank you so much for informing us regarding the error.

Cheers,

Coydir

HR is hard, Coydir is easy

(est. 2022)

Items for the Tester to Verify

❓ Issue duplicate status

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

  • I disagree

Reason for disagreement: [replace this with your explanation]


❓ Issue response

Team chose [response.Rejected]

  • I disagree

Reason for disagreement: [replace this with your explanation]


❓ Issue severity

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

  • I disagree

Reason for disagreement: [replace this with your explanation]


Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants