Conversation
odiff.apply implementationodiff.applyDiffs implementation
odiff.applyDiffs implementation|
Glad you find it useful! I didn't create an apply method because I think most of my uses of odiff in my code just didn't need it. The code you wrote makes sense. The only problem I'm seeing is that there's no way to add items to an array - only to replace the array with an array copy that has those items. If there's any reference to the original array, that reference will no longer point into that object. This is inconsistent with how you've defined applying I think exposing only I'd also recommend separating the path-traversal code so that can be exposed. That would make it easier for people to do any kind of more granular things like replacing arrays you add items to (like your current implementation does). In addition to the above things, before I accept the CL, it needs unit tests and documentation. Would you mind adding that? I actually do have code like your path traversal code (in my private project where I used Note that Thanks for contributing! |
|
Hi! thanks for the detailed reply! As I am using Do you think @fresheneesz that we should continue this PR or perhaps close it until the next good soul comes along? Best |
|
I'm happy keeping this PR open until its finished or we decide its not worth doing. It might be useful to build something that makes it easier to construct custom "apply" functions, so you can apply a diff to something other than a basic object. I'm not sure how that would look tho. I'll leave it your judgement as to whether or not you think this PR is worth continuing. |
Hi!
First of all thank you for
odiff, the differences it produces saved my day at work :DI found myself needing to apply a diff array to an object and I made the implementation in this PR.
Before adding tests and what not I wanted to ask why odiff doesn't have an
applyalready and if the implementation in here makes sense.Thanks!