javaee / json-processing-spec

Legacy JSON Processing spec. Please use the link below to find the current JSON P project
https://github.com/jakartaee/jsonp-api
Other
8 stars 3 forks source link

JsonPatch operations as enum #79

Closed glassfishrobot closed 7 years ago

glassfishrobot commented 7 years ago

the operations are limited 6 operations. it would make it clearer for users if we add them as en enum.

documentation would also be much easier and clearer.

**JsonPatchOperation.java**public enum JsonPatchOperation {

    /**
     * documentation goes here
     */
    ADD,

    REMOVE,
    ...
}

Affected Versions

[1.1]

glassfishrobot commented 7 years ago

Reported by rsandtner

glassfishrobot commented 7 years ago

rsandtner said: operations are defined in RFC-6902

glassfishrobot commented 7 years ago

struberg said: The question is whether a user has to deal with them from 'external' or only inside the impl.

Pro 'external': if you create a JsonPatch from a JsonArray then it might be useful to have them Con: why should someone manually create a JsonPatch via a JsonArray if he could also use a JsonPatchBuilder? Building a JsonPatch from an array is mostly if you get the JSON via a REST endpoint etc, isn't?

glassfishrobot commented 7 years ago

@lukasj said: My current thinking is to have:

public interface JsonPatch {
   enum Operation {ADD, REMOVE,...}
....
}

it has some benefits for providers, so they won't be required to deal with string constants in their implementations and in 'external' case I can imagine ie following use-case:

maybe not a real world example but I think you get the point

glassfishrobot commented 7 years ago

@lukasj said: full patch showing my thinking. Feel free to comment. Thanks!

glassfishrobot commented 7 years ago

rsandtner said: looks good for me

just one point from my side:

instead of switch (name.toLowerCase()) ... you can use

for (Operation op : values()) {
    if (op.getOperationName().equals(name)) {
        return op;
    }
}

throw new IllegalArgumentException("unknown operation);

thanks for pushing it forward

glassfishrobot commented 7 years ago

@lukasj said: thanks! went with your suggestion except for the exception type being thrown, details at: https://java.net/projects/jsonp/sources/git/revision/7731c1e

glassfishrobot commented 7 years ago

File: 0001-JSON_PROCESSING_SPEC-79-JsonPatch-operations-as-enum.patch Attached By: @lukasj

glassfishrobot commented 7 years ago

This issue was imported from java.net JIRA JSON_PROCESSING_SPEC-79

glassfishrobot commented 7 years ago

Marked as fixed on Wednesday, December 7th 2016, 3:30:23 am