search

cctt1014 / pe

0 stars 0 forks source link

Did not handle the case when <amount> is a string instead of a number #5

Open cctt1014 opened 4 years ago

cctt1014 commented 4 years ago

In budget new command, the app did not the case when is a string instead of a number. It just said the command is invalid instead of spotting that the number is invalid. The form of the amount is also not specified in UG.

app screenshot: image.png

UG screenshot: image.png

nus-pe-bot commented 4 years ago

Team's Response

Hi, apologies for not stating that it was only for float values in both user guide and the error command.

It would be low instead of medium in severity as it is not that we did not handle the error, but more of it was just that the error message was too generalised for this case. If the case was not handled, it might crash the system or added errorneous values into the budget. In our case, the string was disregarded and duke manager remains operational.

But it is not a feature flaw but more of a documentation bug as i did not state the constraints of amount. (Would be a huge feature bug if it calculated "k" in current budget)

Thank you for the catch! In future versions of Duke Manager, we will include in the error messages / guides that it only takes in float values in the next release of Duke Manager.

Items for the Tester to Verify

:question: Issue response

Team chose [response.Rejected]

Reason for disagreement: As stated in your response, it is not the expected behavior, so you should not reject this issue.

:question: Issue severity

Team chose [severity.Low]. Originally [severity.Medium].

Reason for disagreement: [replace this with your reason]

:question: Issue type

Team chose [type.DocumentationBug]. Originally [type.FunctionalityBug].

Reason for disagreement: It should should be a feature flaw I think since you did not handle this case. Even if you specify in the user guide, it still need an exception to remind the user.