Changing the Handler interface to differentiate between key and string

[Kosta-Github]

Would it be possible to introduce a slight interface breaking change to the Handler interface?

Consider this example from the SAX documentation:

{
    "hello": "world",
    "t": true ,
    "f": false,
    "n": null,
    "i": 123,
    "pi": 3.1416,
    "a": [1, 2, 3, 4]
}

which creates the following calling sequence for a Handler:

BeginObject()
String("hello", 5, true)
String("world", 5, true)
String("t", 1, true)
Bool(true)
String("f", 1, true)
Bool(false)
String("n", 1, true)
Null()
String("i", 1, true)
UInt(123)
String("pi", 2, true)
Double(3.1416)
String("a", 1, true)
BeginArray()
Uint(1)
Uint(2)
Uint(3)
Uint(4)
EndArray(4)
EndObject(7)

My problem with this is that the String() method is overloaded and used for two different purposes:

1. for defining a string property, and
2. for defining the key for a kay-value-pair inside of the currently active object.

Writing your own Handler needs to keep track of the alternating calling sequence of String() and X where X can be Bool(), UInt(), ... BeginObject(), BeginArray(), and also String() and this even for nested objects!

This could be improved and solved by using another additional method for specifying the key of a key-value-pair within an active object, .e.g:

bool Key(const char* k, SizeType length, bool copy);

The calling sequence from above would change to this:

BeginObject()
Key("hello", 5, true)
String("world", 5, true)
Key("t", 1, true)
Bool(true)
Key("f", 1, true)
Bool(false)
Key("n", 1, true)
Null()
Key("i", 1, true)
UInt(123)
Key("pi", 2, true)
Double(3.1416)
Key("a", 1, true)
BeginArray()
Uint(1)
Uint(2)
Uint(3)
Uint(4)
EndArray(4)
EndObject(7)

This would allow to write much simpler Handler implementations.

But I recognize that this would be a rather breaking API change...

[Kosta-Github]

But if you implement the new Key() method to forward its arguments to String():

bool Key(const char* key, SizeType length, bool copy) { return String(key, length, copy); }

(maybe possible via an std::enable_if?) then the change wouldn't be that disruptive...

[drewnoakes]

An easy patch for existing consumers would be to provide a Key function that just calls String. It's a breaking change, but no hard to recover from.

[drewnoakes]

Haha, snap :)

[miloyip]

This has been asked in stackoverflow as well.

I think the proposal is acceptable.

[pah]

It's a good practice to inherit from BaseReaderHandler to avoid implementing unneeded functions in your handler (its documentation could be improved, though):

template<typename Encoding = rapidjson::UTF8<> >
struct NothingHandler
  : rapidjson::BaseReaderHandler<Encoding, NothingHandler<Encoding> >
{
    bool Default() { return false; } // reject everything
};

So one option would be to add the Key function to this BaseReaderHandler class, forwarding to String, allowing both a user-defined override and a handler providing a single String implementation:

bool Key(const Ch* str, SizeType length, bool copy) {
    return static_cast<Override&>(*this).String(str, length, copy);
}

[miloyip]

Fixed and merged to master in 5e03cbf
Thank you @Kosta-Github