search

caipng / pe

0 stars 0 forks source link

Open command allows duplicate indexes #2

Open caipng opened 2 years ago

caipng commented 2 years ago

Steps to reproduce

  1. Add a person with a valid Linkedin field. Say this person is at index 1.
  2. Enter the command open 1 1 ... 1 1 Linkedin

Expected

Some error is displayed, preventing duplicate indexes OR Only 1 tab/window of that particular Linkedin is opened.

Actual

Multiple tabs are opened, corresponding to the same Linkedin.

Screenshot 2021-11-12 at 4.29.56 PM.png

nus-pe-bot commented 2 years ago

Team's Response

We decided to not implement same-index checking feature, as it could lead to more complicated implementation. Moreover, it's less likely for the user to provide duplicate indexes. Even if they have, it does not lead to any fatal consequences.

It is mentioned in the CS2103/T website:

image.png

Items for the Tester to Verify

:question: Issue response

Team chose [response.Rejected]

Reason for disagreement: I don't agree that this bug should be rejected.

We decided to not implement same-index checking feature, as it could lead to more complicated implementation

I can't agree with the team's choice of not implementing this simple check. There is no use case for this behaviour and this is not a feature. Also, the implementation shouldn't be complicated as checking if the given list of indexes contains any duplicate should be able to be done with a simple for loop.

Moreover, it's less likely for the user to provide duplicate indexes. Even if they have, it does not lead to any fatal consequences.

I agree. This issue won't happen often and even so, only cause a minor inconvenience, which is why I tagged it as a Low severity. However, a bug shouldn't be rejected just because it is unlikely to happen or doesn't cause major inconveniences.

Overzealous Input Validation

I don't feel that this is relevant to this particular bug.

It is better to warn rather than to block when inputs are not compliant with the expected format

The app currently does neither.

Allowing such flexibility can in turn allow the user to use the software in ways you didn't even anticipate

I don't think there's any use case for opening multiple tabs of the exact same webpage

Overzealous rejection of inputs can annoy the user ... However, it is fine (and recommended) to show a warning for such inputs to guard against the deviation being a mistake rather than intentional.

The app doesn't reject the input nor show a warning.

Hence, I don't think that the concept of "overzealous input validation" is applicable here, as the app does not validate this input.

Overall, I will maintain that this should be a classified as a low severity functionality bug as in the original issue.