search

JustinSoh / pe

0 stars 0 forks source link

Inputting an Invalid Current Semester with the 'set' command - the error is not clear #1

Open JustinSoh opened 2 months ago

JustinSoh commented 2 months ago

Description: The error output when a current semester is invalid is too vague.

Steps to Reproduce:

  1. launch the jar file
  2. set n/test curr/10

Actual Result: Error states "Invalid set command: Invalid currentSem format/order Type "help" to view the list & syntax of available commands"

It should have some form of error telling me what is considered a valid semester

Screenshot: image.png

nus-pe-bot commented 2 months ago

Team's Response

"It(error message) should have some form of error telling me what is considered a valid semester"

Do agree that it could be vague on what the error message is talking about, but the error message shown is exactly what should be understood: currentSem format or the order is invalid. As seen in the UG, the format for <CURR_SEM> should be an integer value from 1-8

In the UG as well, written as a IMPORTANT note: IMPORTANT: All arguments must be provided and must follow the specified order in Format

Moreover, it suffices that in our scope this error message guides users to know what they are typing wrong by telling them to use the help command to see the required syntax Error message: Invalid set command: Invalid currentSem format/order. Type "help" to view the list & syntax of available commands which will guide the user to type help, and see the correct syntax and argument format for the set command.

On this note, future iterations will attempt to be more user friendly in the way the error message is printed out to make the error known clearer and let user know what is wrong. For now, we will put it under out of scope because this is not a priority in v2.1.

Items for the Tester to Verify

:question: Issue response

Team chose [response.NotInScope]

Reason for disagreement: For instance,

  1. set n/test curr/10 is wrong as per the user guide as the value is out of bound
  2. set n/test curr/a is wrong as per the user guide as only integers are allowed

However both of this input gives the same error output, and when I used the help command - I do not get any additional insight on how I might fixed it. Yes, I could refer to the User Guide but as per the 2113t website, vagness of error message is considered a feature flaw.

image.png

As such, I disagree with the NotInScope error as this is a feature that you have already considered (since it is indicated in your User Guide).

Warmest regards, Tester