nus-cs2103-AY2324S1 / pe-dev-response

0 stars 0 forks source link

Too restrictive constraint of add command #4879

Open nus-se-script opened 11 months ago

nus-se-script commented 11 months ago

Given that I am able to add a year of 0, which represents advance placement, I should be allowed to exclude the semester parameter as it will not have any impact on the command. Given that this application should benefit fast typist, it might be better to allow the omitting of the semester parameter specially for year 0.

image.png`


[original: nus-cs2103-AY2324S1/pe-interim#4074] [original labels: severity.Low type.FeatureFlaw]

marquestye commented 11 months ago

Team's Response

We agree that a perfect implementation would probably exclude the need for a semester parameter when adding to advanced placement.

It was implemented this way as the edit command requires a semester to be present and preserved.

However, the fixing of this feature flaw is not essential for the app to be reasonably useful, as it is just an additional semester parameter. The feature could have been implemented in a better way, which would be as a whole new command specifically to add advanced placement modules (because the add command requires parsing of the semester). However that would require much additional effort, also because the edit command would also have to check for this special case, and have special error messages for these scenarios.

As such, as per:

image.png

we feel that this classifies as Response.NotInScope.

Duplicate status (if any):

--