search

lwlshawn / pe

0 stars 0 forks source link

Parameters cannot be supplied in any order for delete command #4

Open lwlshawn opened 2 years ago

lwlshawn commented 2 years ago

image.png

The UG states that parameters for an argument can be given in any order, however this does not appear to work for delete, as shown above.

nus-pe-bot commented 2 years ago

Team's Response

Although the UG does state that parameters can be given in any order, the parameter does include the flag. Furthermore, the error message is very clear in saying that the flag should come before the index if the visit is to be deleted. It is not realistic to expect the delete command to handle cases where the index may come before or after the v/ flag, since it is ambiguous. This is why we did not allow it and instead showed an error message.

Items for the Tester to Verify

:question: Issue response

Team chose [response.Rejected]

Reason for disagreement: In their UG, they explicitly state the following:

image.png

As seen in the picture, any words in UPPER_CASE are parameters. Now, in the documentation of their delete command:

image.png

INDEX here is in upper-case, so clearly it is meant to be a parameter. And finally, in this part of their documentation:

image.png

They say that parameters can be given in any order, thus a reasonable reading of their UG shows that this is a bug. They say "they do not mean the index" but according to their own documentation INDEX is clearly a parameter. The examples provided are irrelevant, given that they themselves have stated that parameters should be possible to supply in any order.

In their response they have tried to argue that "It is not realistic to expect the delete command to handle cases where the index may come before or after the v/ flag, since it is ambiguous". This seems untrue, and I would like to justify my standpoint here. In the code, all that would be required is to check the command for the presence of the v/ prefix first, before processing the index. This is a very simple check to implement, and would have fixed this problem.

:question: Issue severity

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

Reason for disagreement: I think this qualifies for medium, as it is sufficiently likely to come up at some point for experienced users who are very fast with the application, and used to being able to freely re-arrange the parameters for other commands, when they suddenly run into issues with this particular command.