JsonPatch icon indicating copy to clipboard operation
JsonPatch copied to clipboard

Breaking change: keep original casing (uri & property names) when serializing

Open KevinDockx opened this issue 8 years ago • 5 comments

KevinDockx avatar Jul 04 '17 13:07 KevinDockx

@KevinDockx this wouldn't take me very long to implement. I can send a pull request if you are interested.

Since you're talking about breaking changes, what do you think about combining the multiple ctors on JsonPatchDocument<T> into a single ctor with optional arguments?

leemicw avatar Jul 06 '17 14:07 leemicw

@leemicw It could help by creating a interface (like the IContractResolver in NewtonSoft) and from there let the object that will change the case to inherit from the interface and each one will do its own stuff.. for example:

interface ICaseTransform {
    string Transform(string s);
}

class CamelCaseTransform : ICaseTransform {
    public string Transform(string s) {
        return string.Empty; //transform to camel case
    }
}

When I did the changes to support the camel case I made the quick change by using enums, but if you want to follow the SOLID principles it is better to use the interfaces.

mtzaldo avatar Jul 10 '17 02:07 mtzaldo

@mtzaldo The interfaces sounds fine to me. When I hear back from Kevin I will work up something.

leemicw avatar Jul 14 '17 18:07 leemicw

@leemicw: PR's are always welcome :)

As far as changing the enum to that interface implementation: I agree on following SOLID principles, but I'm a bit hesitant to break the existing (enum-based) implementation. Maybe the enum-approach could be marked as obsolete?

On getting rid of ctor overloading: I'm wondering what the added value of that would be?

KevinDockx avatar Jul 17 '17 10:07 KevinDockx

@KevinDockx going to a ctor that looked something like this

JsonPatchDocument(List<Operation<T>> operations = null, IContractResolver contractResolver = null, CaseTransformType? caseTransformType = null)

If the parameter is null use the default value. So calling new JsonPatchDocument<MyObject>() still builds up all the default values. This keeps from having to maintain all the possible ctor combinations.

If you think we should keep things the way they are I am fine with that. Just thought I would bring this up as an option.

leemicw avatar Jul 31 '17 15:07 leemicw