Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor JSON Pointer code to be more self-contained #38

Closed
tatu-at-salesforce opened this issue Feb 13, 2013 · 3 comments
Closed

Refactor JSON Pointer code to be more self-contained #38

tatu-at-salesforce opened this issue Feb 13, 2013 · 3 comments
Labels

Comments

@tatu-at-salesforce
Copy link

It would be nice to make existing JSON pointer more self-contained, to make it easier to reuse -- I am hoping to try to build JSON Pointer support for Jackson, either in core or as add-on module.

@fge
Copy link
Collaborator

fge commented Feb 13, 2013

Agreed, the current code has many adherences which make it uneasy to reuse:

  • a heavy dependency on Guava;
  • all token decoding/encoding into the class itself;
  • "tree resolving" into the class itself as well.

And tree resolving is heavily JsonNode-dependent.

And as a more general rule, the code is quite complicated. As a first step, the plan is to split this class into two disctinct classes:

  • ReferenceToken, which would represent one token of a JSON Pointer;
  • JsonPointer which would just be a Collection<ReferenceToken>.

This would simplify matters a lot:

  • JSON Pointers are always split according to '/', so a simple .split('/') is enough to grab tokens, no need for Guava's Splitter;
  • it would be the role of ReferenceToken to err out in the event of a malformed token, by throwing an exception -- which can be changed if the code is salvaged;
  • ReferenceToken's .toString() would then output the cooked form of a token, so that JsonPointer's .toString() would just look like:
@Override
public String toString()
{
    final String ret = "";
    for (final ReferenceToken token: tokens)
        ret += '/' + token;
    return ret;
}

That is not all: the current class' .isParentOf() (to be renamed) would still work fine if ReferenceToken's .equals() and .hashCode() just use this token's cooked representation -- while its .resolve() method would use the raw one.

So: code simplification, more portable and adaptable. A win win. Will do it.

@fge
Copy link
Collaborator

fge commented Feb 13, 2013

This leaves one question open, however: even with this refactoring, the code will be dependent on JsonNode. Especially given the special rules of a JSON Pointer with regards to an array:

  • any numeric value starting with 0 and which representation is not exactly 0 is not a legal array index (ie, 0 is the first element of an array, 00 in the event of an array is ENOENT);
  • what of other tree formats which are not JSON?

Meh. I'll do a first implementation and then we'll see what's what.

@fge
Copy link
Collaborator

fge commented Feb 16, 2013

See here:

java-json-tools/json-schema-core#1

@fge fge closed this as completed Feb 16, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants