search

gerteck / pe

0 stars 0 forks source link

Edit Command throws misleading output error #2

Open gerteck opened 4 months ago

gerteck commented 4 months ago

According to UG,

image.png

one cannot change the NRIC using the edit command. This is acknowledged. However, when I try to edit the NRIC,

image.png

It says that the NRIC is invalid, 

 should be @xxxxxxx# where @ is a letter that can be S,T,F,G or M and # is the appropriate letter.

I believe this is a wrong error message as in thesescenarios:

  1. wants to change existing users NRIC, gets confused as it does not throw message saying that it cannot be edited, but says that NRIC is invalid instead. Perhaps it would be better to give suggestion, by detecting same name and saying: ' if u want to change nric, do XXX'

  2. mistypes NRIC that the person wants to change, in which case the error message is wrong as the NRIC is indeed valid, it is instead the NRIC is not found in the database.

  3. the IC is not a valid IC in terms of checksum, it is still valid format, but wrong checksum.

nus-se-script commented 4 months ago

Team's Response

You cannot have an error message where you satisfy users looking to change the name and users looking to change the NRIC.

The documentation already provides the appropriate instructions if they are looking to change the NRIC.

Items for the Tester to Verify

:question: Issue response

Team chose [response.Rejected]

Reason for disagreement: Apologies for the confusion. Agree with the team that

You cannot have an error message where you satisfy users looking to change the name and users looking to change the NRIC.

I did not read the documentation clearly, and thought that one could change the NRIC using the edit command. As the behavior differed from what was expected in the AB3, I had wrongly associated the command to the behavior. That is a fault on my part.

However, I still believe that

the IC is not a valid IC in terms of checksum, it is still valid format, but wrong checksum.

This point is still valid. I feel strongly for this point as I faced a difficult time during the PE trying to understand what was wrong, as I did not know that NRIC implementation had to pass the checksum, especially since the NRIC I had provided was well within what the NRIC validity error message specified. So I think the output error can be better in that sense, and throw an appropriate error when it is the NRIC that is wrongly formatted, instead of saying it is invalid, since it is valid based on the format shown:

Sample Input NRIC: S2234567D Error Message: NRIC invalid! Should be @xxxxxxx#

However, the NRIC clearly matches the format given.