Tixit / odiff

Gets a list of differences between two javascript values.
MIT License
88 stars 13 forks source link

Option to get all differences instead of two changes or less than 10% #29

Closed CL-SebasVelez closed 3 years ago

CL-SebasVelez commented 3 years ago

Hi guys, I added an option received by param to get all the differences instead of two changes or less than 10%. I want to know why they limit this option to just two changes or 10%. Are there reasons for this? My reason is that I need to get all the properties of the object when it is updated.

fresheneesz commented 3 years ago

Hi Sebastian, thanks for putting together a pull request! So that part of the code is intended as an optimization. The similar function determines what kind of difference to create. Whether or not similar returns true or false, all the differences are still returned, just potentially in a different form. The "Algorithm Behavior" section talks about this:

If an array element is changed only a little bit, a sequence of change items is generated to change that element. A "little bit" is defined as less than two shallow inner values being changed or fewer than 10% of its shallow values being changed. If an array element is changed a lot (defined as the opposite of "a little bit" above), a single change item will be generated to reset that complex value. This is intended as a trade off between number of changes and "size" of each change.

The reason that arrays of size 2 or less are treated as "not similar" is because there is no major difference in how the algorithm would output the differences with arrays that are that short. This is very much related to servicing the operation of the differencing algorithm. So I can see that if you're trying to use the similar function directly, the definition that excludes arrays of 2 or fewer items might not be desirable. But that doesn't seem to be what you're doing since you mentioned you're trying to "get all the diffs". All the diffs will be returned regardless. So maybe I don't quite understand your use case where this behavior is problematic.

Your patch looks like it does a lot of reformatting of the file. If I were going to accept a patch, I would want something that's cleaner and easier to review. However, let's nail down what should happen in this case before putting more effort into the patch.

CL-SebasVelez commented 3 years ago

Hey, thanks for taking the time to answer me. I don't think I give to you a good explanation. I will close this PR and create another one with a simple change that I made because I used a Prettier and it modified all the spaces.