Open PetrichorPrecipice opened 7 months ago
Although it might seem overtly unnecessary and complicated, we intentionally designed these commands in this way. This is to ensure all our employee commands are standardised; to have prefixes before the respective input parameters as long as there is more than one parameter involved. This is with the exception of the edit command, which uses the parameter INDEX
, since the employee ID is an attribute of the employee which can be edited as well. By ensuring that the commands of the application are standardised, this reduces confusion for the user, who will not have to wonder if a certain command or parameter requires a prefix to be entered.
Nonetheless, we did consider this issue in our implementation, which is why commands accepting only the EMPLOYEE_ID
parameter do not require the id/
prefix, for example delete
and report
.
Removing EID
from EMPLOYEE_ID
might end up being counterintuitive, as this might be confused with other parameters like phone number, which is also 8 digits long.
Therefore, we would classify this as NotInScope, since it is a valid issue, but is less important (based on the value considerations) than the work that has been done already.
Team chose [response.NotInScope
]
Reason for disagreement: I think this is pretty important and not that hard to do. I think that removing EID from employee_id is not something that should have taken an extensive amount of work but would have nevertheless made the user experience easier.
For commands like addremark and deleteremark, it is possibly redundant to include having to type EID after already having the id/ parameter in the command itself. Perhaps one or the other could be removed and concatenated in the program itself.