search

OungKennedy / pe

0 stars 0 forks source link

Undo command rolls back more than 1 step #5

Open OungKennedy opened 4 years ago

OungKennedy commented 4 years ago

How to replicate: 1.Add 2 types of stocks 2.use undo

  1. use "list stock"
  2. Observe that list is empty

image.png

nus-pe-bot commented 4 years ago

Team's Response

Accepted. This bug was not caught by our team, because we were testing the undo and redo function on a MacOS. After this bug was highlighted, we tried to determine the cause of the problem.

Bug Status

| Operating System | Java Version | Bug Status|

|----|----|----|

| Windows 10 | Java 11.0.4 | Undo and Redo lead to loss of data and concatenation of distinct statelist elements. |

| MacOS El Capitan | Java 11.0.4 | No bug. |

| MacOS Catalina | Java 11.0.4 | No bug. |

| Ubuntu 18.04 | Java 11.0.4 | No bug. |

Cause

The bug occurs because we used System.getProperty("line.separator") to split our line of our state data string. However, when this state data is created, it is created by explicitly appending a "\n". However, when reading the string, using System.getProperty("line.separator") causes an error in Windows, because in windows line separators are "\r\n" instead of simply "\n". Here is a link to explain this difference in how DOS handles line separators compared to UNIX bases systems such as the MacOS

Link : UNIX vs DOS Line Seperator.

Fix

I also managed to fix this issue by changing all instances of System.getProperty("line.separator") to "\n" as shown in the screenshot below.

See lines 29, 34, 55. This fixes the issue because now, the state data string is read exactly the same way as it was saved, regardless of operating system.

Screenshot 2019-11-18 at 2.29.04 PM.png

This issue also affects the Redo command which is dependent on the Undo command

Duplicate Issues

I have set all the related undo and redo issues which broke due to this problem as a duplicate of this issue. By applying the fix shown below, both the Undo and Redo commands will work perfectly on both MacOS and Windows. Currently all the duplicated issues are not reproducible on Unix based operating systems such as Mac OS or Ubuntu. But on Windows 10 it will lead to the bug described above. The solution above fixes all of these issues completely.