Bisceto / pe

0 stars 0 forks source link

Parameters can be left empty when entering commands #7

Open Bisceto opened 1 year ago

Bisceto commented 1 year ago

I tried to create a new module with the following command "add CS2103 /name /tags" and it was allowed. Not sure if this is meant to be the intended behaviour, but perhaps an error message should be provided to let the user know that they have left some of the parameters blank. If they were to key such a command, it would most likely mean they forget to key in the actual name or tags, so an error message should catch it for them.

soc-se-bot commented 1 year ago

Team's Response

First, just to clarify, being able to provide no values to the name and tags arguments for the add command is an intended behaviour of the application. A module can be nameless and can also have no tags.

In response to the point you raised,

"it would most likely mean they forget to key in the actual name or tags, so an error message should catch it for them"

That is something we did take into account when developing this command. Ultimately, we decided that treating it as an error and showing an error message would be considered overzealous input validation. Our reasoning is that there could be such a scenario:

  1. User wants to add a bunch of modules, some with names and some without, and some with tags and some without
  2. User enters and executes the command to add the first module: add GEA1000 /name /tags
  3. Instead of typing the full command from scratch, user presses the up arrow to cycle through previous commands (this is our command history feature) and now has add GEA1000 /name /tags in his command box
  4. User edits the details of the command in order to add the second module: add CS2103 /name Software Engineering /tags Programming
  5. User repeats step 3 and 4 for the other modules he wants to add

This is essentially a scenario where the user is using add GEA1000 /name /tags as a sort of base/template and modifying it for each module. As a Linux user who uses a lot of CLI based applications, this is something I do quite commonly. It provides the advantage of not having to retype the whole command over and over and also reduces the chances of typos in the argument names. As such, treating such a command as an error and showing an error message would be an inconvenience to the user and might be overzealous input blocking.

That said, we do see some merit in the point you have made. Perhaps, showing a warning message but still letting the add command pass through and be executed would be a more ideal behaviour. For example:

Warning: The /name and /tags arguments were provided without value! If this was a mistake, please use the "edit" command to rectify.

However, we feel that this would be considered a very low priority feature. To implement this feature, we would likely want to review all our arguments across all commands to be consistent. Hence, we feel that rectifying it is less important (based on the value/effort considerations) than the work that has been done already. To add on, the current feature does not cause any unintended error messages or crashes. Thus, we will categorise this as "NotInScope".

Items for the Tester to Verify

:question: Issue response

Team chose [response.NotInScope]

Reason for disagreement: [replace this with your explanation]