XihuaZ / pe

0 stars 0 forks source link

Inconsistent date parameter for `add` command for transactions #4

Open XihuaZ opened 12 months ago

XihuaZ commented 12 months ago

Adding transactions for on/10/10/33, on/10/10/43 and on/10/10/83 causes inconsistent date behaviour. With on/10/10/20, on/10/10/33 etc., they year is deemed as year 2020 and year 2033 respectively, however, with on/10/10/83, it is deemed as year 1983.

command to reproduce error:

add ty/R d/Sold 1 Mug amt/10 on/10/10/33 s/1
add ty/R d/Sold 1 Mug amt/10 on/10/10/43 s/1
add ty/R d/Sold 1 Mug amt/10 on/10/10/83 s/1

Screenshot 2023-11-17 at 4.37.07 PM.png

Screenshot 2023-11-17 at 4.36.26 PM.png

nus-se-script commented 11 months ago

Team's Response

Yes, it would be clearer if we used yyyy format but we optimized for cli usage and deemed that it is not usually the case the user needs to specify the first 2 digits of the year.

Items for the Tester to Verify

:question: Issue response

Team chose [response.NotInScope]

Reason for disagreement: The response of the dev team does not address why the first 2 digits can deviate based on what the last 2 digits are.

This is clearly an error with the code for parsing the Date attribute, as it does not function as expected. The expected year should always be of 20XX, with only the last 2 digits being customizable. However, in this case putting 83 causes the year to be reflected as 1983 instead of 2083. This is clearly an unexpected behavior that should not be present since the first 2 digit of the year is supposed to be fixed. Hence, it does not classify under response.NotInScope.

Information on the fixed first 2 digits can be seen from the paragraph below. There are no mentions of how or why the year within the date attribute could deviate.

Screenshot 2023-11-23 at 10.13.12 PM.png


## :question: Issue severity Team chose [`severity.Low`] Originally [`severity.Medium`] - [x] I disagree **Reason for disagreement:** The response of the dev team does not address why the first 2 digits can deviate based on what the last 2 digits are. They also did not provide reason for the drop in severity. You have mentioned in a separate issue that this app could also be used for planning of future transaction in your `Team's response` section in a separate issue [#28](https://github.com/nus-cs2103-AY2324S1/pe-dev-response/issues/28). The quote: > Our software is designed to accommodate the recording of foreseeable transactions, not just those that have already occurred. This feature is particularly useful for planning and recording recurring or scheduled transactions, such as monthly salaries, subscription fees, or anticipated sales. The flexibility to enter future-dated transactions allows users to better manage and foresee their financial activities, offering a comprehensive view of both past and upcoming transactions. For any individual utilising this app for doing future projections, this app would fail when handling years beyond a arbitrary point in time. This would cause severe issue for this group of individuals since the app would cause the year to be reflected as in the 19XX year instead. Furthermore, as the switch to 19XX is arbitrary since adding year `2024`, `2033` and even `2043` is possible but not year `2083` (recognised as `1983` instead as shown in image below). (Note that the arbitary switch into 19XX does not occur only from `83` onwards for year in `date` attribute, and could start occurring much sooner.) ![Screenshot 2023-11-23 at 11.57.45 PM.png](https://raw.githubusercontent.com/XihuaZ/pe/main/files/9662bd80-f646-48ca-a295-a73a7bc94dff.png) However, they would not know at what point would the app would take year input and assume the input year to become 19XX. Thus, this would cause huge inconvenience for them.