WeeeHung / pe

0 stars 0 forks source link

Sorting does now do alphabetical order as expected #5

Open WeeeHung opened 8 months ago

WeeeHung commented 8 months ago

The Feature sortStu o/name r/1 says that i can sort the students names in alphabetical order, be it reverse or not.

But this does not handle case sensitivity. In this case, i have a student named abC and another student named ABC. The are not placed together as tied in alphabetical order but separated as the implementation probably compared the string based on ASCII or some hash values.

image.png

[Note student 6 and student 1]

nus-pe-script commented 7 months ago

Team's Response

Thanks for your feedback! However, this case sensitivity is intentionally left here for two reasons.

First, we want to keep the consistency of all alphabetical search to avoid any potential confusion so we make all alphabetical sort to be case sensitive.

Second, user usually have their preferred style to key in the name. Some people may prefer the first letter to be capitalized while some perfer the first letter not to be capitalized, this is why we did not enforce a first letter check for the name. However, assume one user prefers capitalizing first letter, in this case, it is still possible to not capitalize the first letter due to typos. Hence, case sensitive name sort can allow them to quickly identify those names which they forgot to capitalize the first letter so that they can edit accordingly to have a name consistency.

In addition, I believe the case sensitivity is less important than the work already done in the find feature and this does not affect user experience much.

However, we understand that some people may be more comfortable with case insensitive name search and we may include this in the future iteration and allow the user to choose the case-sensitivity of the name search.

Items for the Tester to Verify

:question: Issue response

Team chose [response.NotInScope]

Reason for disagreement: Thank you for the response. Perhaps the example i gave previously was not enough to show the impact of mishandling case-insensitivity for sorting. I'll try to show a better example on why this defeats the purpose of sorting student list which could easily contain hundreds of students.

As mentioned in the UG, which I think is a valid user expectation, when I do sortStu to get a list of students, I expect them to be in alphabetical order, be it ascending or descending.

One possible use case as a professor using this application for module management is that, he/she may want to get a sorted list of students taking this module in alphabetical order and set it as the seating arrangement for an examination. Since the sorting is not only implemented on the first letter, if there are capital letters in a students Name (regardless of position of this alphabet), the sorted list could be messed up. In such a case, the alphabetical order is wrong and it was not due to typos, but mishandling from the app side. An example will be shown below, where I did a sortStu command:

image.png

Expected sequence (as a < b < c):

  1. Leanne

  2. LeBron James

  3. Leca Mary

Actual sequence:

  1. LeBron James

  2. Leanne

  3. Leca Mary

There could be a simple fix for this by setting names to uppercase before sorting, hence, additional overhead and effort for implementation would be invalid as a reason.

I think this is a valid bug that did not address possible sorting issues and could trouble users. For instance, like the team have mentioned,

However, assume one user prefers capitalizing first letter, in this case, it is still possible to not capitalize the first letter due to typos. Hence, case sensitive name sort can allow them to quickly identify those names which they forgot to capitalize the first letter so that they can edit accordingly to have a name consistency.

I think this might not be that simple as it could require users scrolling down a long list of student list to realize the capitalization inconsistency. Furthermore, the edit name to maintain consistency and find seem more like troubleshooting, and that would not have been needed if case insensitivity is implemented for sorting (aka. capitalisation consistency would not have been that important if the app already addresses it under logic). Thank you once again and I hope I managed to get the message across this time.