Allow items() to be used with custom string

[crazyjul]

When using an alternative string, the iteration_proxy_value is not compatible, as it returns const std::string &. I replaced it by the type given string.

I made a function to abstract std::to_string, but I don't really like the way it's done and would like your opinion.

---

Pull request checklist

Read the Contribution Guidelines for detailed information.

• Changes are described in the pull request, or an existing issue is referenced.
• The test suite compiles and runs without error.
• Code coverage is 100%. Test cases can be added by editing the test suite.
• The source code is amalgamated; that is, after making changes to the sources in the include/nlohmann directory, run make amalgamate to create the single-header file single_include/nlohmann/json.hpp. The whole process is described here.

Please don't

• The C++11 support varies between different compilers and versions. Please note the list of supported compilers. Some compilers like GCC 4.7 (and earlier), Clang 3.3 (and earlier), or Microsoft Visual Studio 13.0 and earlier are known not to work due to missing or incomplete C++11 support. Please refrain from proposing changes that work around these compiler's limitations with #ifdefs or other means.
• Specifically, I am aware of compilation problems with Microsoft Visual Studio (there even is an issue label for these kind of bugs). I understand that even in 2016, complete C++11 support isn't there yet. But please also understand that I do not want to drop features or uglify the code just to make Microsoft's sub-standard compiler happy. The past has shown that there are ways to express the functionality such that the code compiles with the most recent MSVC - unfortunately, this is not the main objective of the project.
• Please refrain from proposing changes that would break JSON conformance. If you propose a conformant extension of JSON to be supported by the library, please motivate this extension.
• Please do not open pull requests that address multiple issues.

[coveralls]

Coverage remained the same at 100.0% when pulling 4615f5a on crazyjul:fix/items-with-alt-string into 99d7518 on nlohmann:develop.

[jaredgrubb]

FWIW, looks good to me! Thanks for the tests too :)

[t-b]

I dont' think the introduction of a custom int_to_string function is a good solution. Why not call it to_string and let the compiler choose the correct implementation from either the one in std namespace or a user implementation in its own namespace found via ADL?

https://stackoverflow.com/a/50555639 has an example.

[crazyjul]

It's not exactly the same here. The example shown is about creating a to_string for other types. Here, I need a to_string that returns other type of string, the alt string.

There's no easy way to select function on return value in c++. It tried something like this :

using namespace std;
(static_cast<string_type ( int ) >( to_string ))( array_index )

But it doesn't find the to_string for alt_string, as the iterator is in a detail namespace.

If you have ideas to fix this, I'm open

[jaredgrubb]

As @crazyjul says, ADL only kicks on argument types, not on return types as you suggest. Therefore the custom type has to be one of the args. I don't see any other way to do it than to make it void bikeshed_name(custom_string_type&, int). I think the proposed patch is the right solution.

[t-b]

@crazyjul @jaredgrubb Thanks for the answers. Yes indeed ADL does not help here.

But still forcing the user to implement this function does not sound like the right approach. But I'm also running out of ideas how to do it better.

[crazyjul]

There two other solutions :

• force the alt_string to have a copy constructor / operator to std::string
• go to char * with std::to_string( row ).c_str()
  Both those solution would add a memory allocation, but would remove the need for that extra function.

There's other solutions requiring some templates, that would allow user to choose that default mode, but be able to override with a function of his own. I can make some proposition, but I need to experiment first

[t-b]

@crazyjul Let's hear first what @nlohmann thinks of the current state before you start putting more effort into it.

[jaredgrubb]

What if you do:

template <typename T>
void int_to_string(T& str, int value) {
    str = std::to_string(value);
}

// ...

      if (array_index != array_index_last)
      {
              using detail::int_to_string;
              int_to_string( array_index_str, array_index );
              array_index_last = array_index;
      }

That provides a default implementation that only requires that the custom string type is convertible from std::string, but also provides a ADL-searchable customization point.

[crazyjul]

Done ;-)

[nlohmann]

Looks good to me.

[nlohmann]

Thanks!

[nlohmann]

---

🔖 Release item

This issue/PR will be part of the next release of the library. This template helps preparing the release notes.

Type

• ✨ New Feature
• 🐛 Bug Fix
• ⚡️ Improvement
• 🔨 Further Change
• 🔥 Deprecated function

Description

• items() function can be used with custom string types.

---