Allowing comments

[devlaam]

Hi Erik,

Very nice piece of work indeed!

Recently i changed from play to your parser for my home brew json operator lib. Because I have quite a lot if "comments" in my json i forked your lib, and wanted to extend it so it can spit out comments or at least skip them. Due to the clarity of your code this is rather easy to do.

I have a question about one design decision you made though. You differentiate between parseStringSimple and parseStringComplex, the only difference (from a performance point of view at least) seems to be the inspection of \\. The price you pay is rescanning the string when it turns out not be simple after all. Was this the sole ground for have two separate methods here, or am I missing something? Would one extra if statement in the scan make a big difference and not outweigh the disposal of work done?

If not, i would integrate the methods scan regularly until the first \\ and then, if this happens, switch to the collection into the CharBuilder. But, maybe you have already tested this and turned out to be a dead end.

[non]

@devlaam Keeping parseStringSimple as simple as possible does make a big difference in performance, so adding an if there is not something I'd want to do.

However, your comment gave me an idea: instead of just using -1 to signal failure, we could use -1 through Int.MinValue to specify exactly how far we got. That would mean we could quickly add all that data to the builder without having to check it, and might make a big difference.

Thanks for your comment! I'm going to try this out and see if I can notice an improvement in benchmarks.

[non]

java.util.Arrays.binarySearch uses a similar trick: https://docs.oracle.com/javase/7/docs/api/java/util/Arrays.html#binarySearch(byte[],%20byte)

[devlaam]

Nice! Glad to be of service. In the mean time i forked your repro and made the adjustments for my use case. As speed is not my most importent goal i made a few changes as well and integrated the methods Simple/Complex into one method. A pull request seems to make less sense given the spirit of the answer you gave above, but i thought i'dd publish it anyway. Maybe others have similar use cases.