darkmoongreatsword / pe

0 stars 0 forks source link

`advfilter` doesn't account for mixture of alphabetical characters and numbers #3

Open darkmoongreatsword opened 1 week ago

darkmoongreatsword commented 1 week ago

advfilter is unable to tell if the input value should be interpreted as a string or a numerical value.

Screenshot 2024-11-15 at 4.54.09 PM.png

Screenshot 2024-11-15 at 4.54.29 PM.png

When there is a mix of numerical values and alphabetical strings as tag values, suppose due to a company registering their staff with a mix of numbers and alphabetical characters, advfilter does not recognise that the user may wish to use a numerical value as a string to compare.

Here, as a user, I am expecting that all 3 of the people added should be shown since I am comparing against the string '0'.

nus-pe-bot commented 4 days ago

Team's Response

We designed advfilter with the intent on parsing the user's input as a double where possible and a String (lexicographically compared) if not. If it is not parsed as doubles, there can be edge cases where the String is not parsed as a number (even though it's possible) that result in undesired behaviours. Thus, advfilter does compare based on doubles first before Strings, which we believe is of greater value to the user, than to just compare as Strings only. Additionally, if the implementation had been to only parse the input as Strings, the user would see many unnecessary and unmatching/irrelevant values in the advfilter result, and less technical users may even consider this a large functionality flaw as it would slow them down (defeating the purpose of CLI-first) or distract them unnecessarily as they may be unaware of lexicographical comparisons of Strings. Thus, we believe that this implementation is a better match for our target users' profile. However, all that being said, we acknowledge that this behaviour could have been mentioned in the User Guide, and thus mark this as a Documentation bug.

Items for the Tester to Verify

:question: Issue type

Team chose [type.DocumentationBug] Originally [type.FeatureFlaw]

Reason for disagreement: I would actually characterise this as a Functionality Bug. According to the CS2103T guidelines for bug reporting, if the actual functionality and the user guide differ, it can be considered a functionality bug.

It satisfies the following: User guide states that alphanumerical values like '223302B' are accepted as valid input. However, the actual functionality of the application rejects this valid input.

Screenshot 2024-11-21 at 8.55.01 PM.png


## :question: Issue severity Team chose [`severity.Low`] Originally [`severity.Medium`] - [x] I disagree **Reason for disagreement:** I will agree that this bug could be either a flaw in documentation or its implementation. In this case, I accept that the team recognises it as a documentation bug. Even still, I will maintain that this bug is of medium severity. Especially since the team used the term 'alphanumerical values' multiple times to describe valid tag values in the UG. It even shows up as an example tag value. This is taken from the section "Adding a contact: `tag`" from the BAE User Guide. ![Screenshot 2024-11-21 at 8.46.26 PM.png](https://raw.githubusercontent.com/darkmoongreatsword/pe/main/files/1f625271-7226-4c48-9bf6-bc77cfb03777.png) ![Screenshot 2024-11-21 at 8.43.30 PM.png](https://raw.githubusercontent.com/darkmoongreatsword/pe/main/files/68d798d6-88f6-4aca-97de-1159e42553ef.png) If this is not the intended behaviour of the tag function, then this is a very misleading section as it clearly tells the user that an invalid input is valid. Based on the guidelines for bug severity, since your team has shown that a combination of alphabetical characters and numbers is a valid and common in practice string, I do not believe that this can be considered as a rare situation. Hence, I have stuck with medium severity for this bug. ![Screenshot 2024-11-21 at 8.50.39 PM.png](https://raw.githubusercontent.com/darkmoongreatsword/pe/main/files/5f2c3ac7-fca4-4a83-bb66-192f6e6661fa.png)