String conversion functions / methods

[mikee47]

There are numerous places within the framework where we have functions and methods to convert typed values into strings. A few examples:

• NanoTime::Frequency::toString() and other classes
• IpAddress::toString()
• MacAddress::toString()
• Ssl::getCipherSuitName(CipherSuite id)

It appears this is not the most appropriate way to do this:

http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c4-make-a-function-a-member-only-if-it-needs-direct-access-to-the-representation-of-a-class

They should, in fact, be declared as regular functions. This one's got it right (not mine!):

• ContentType::toString(MimeType mime)

I can see the sense of this. It would make for a much more uniform API just typing toString( then getting a selection of type conversions in the IDE. It also avoids making up names for things.

There are proabably other conversions which could benefit from this appropriate, but perhaps if there is agreement we can add toString() functions. In many cases to avoid breaking things they'd be in addition to existing class methods, but if they've been added since 4.0.1 release we can just replace them.

What do you think? Worth doing?

[mikee47]

Forgot to mention, cruicially this would work uniformly with any type of data, including enums which cannot of course have methods.

[aemseemann]

I like the idea, in particular because the standard library is also moving into that direction with std::size(...) and friends. It also resembles the common pattern of having overloads of operator<<(std::ostream& s, ...) for all types that may need a textual representation.

Before this is rolled out to the entire code base, an aspect to consider is which return type to allow for toString. Always a Wiring String or would it be fine to return a const char * or FlashString, too? Shouldn't matter in most cases thanks to implicit conversion constructors, just method chaining constructs like toString(...).otherMethod() would not work universally.

[mikee47]

IMHO best to standardise on WString for consistency, and as you mention method chaining (although I'm not a real fan of that idiom). In actual use it'll most likely end up in a WString expression anyway. Whilst const char* is convenient for use in printf expressions I'm trying to move away from using it (except in debug_x calls) in favour of Print methods as it's more robust.

It should also be clear that, IMHO, toString should produce a readable string. For example:

using BinaryData = std::array<uint8_t, 16>;

String toString(const BinaryData& data)
{
  return makeHexString(data.content()
}

Whilst we could also produce a String object from the binary data, that's should be done by something else:

String toBinaryString(const BinaryData& data)
{
  return String(reinterpret_cast<const char*>(data.data(), data.size());
}

[mikee47]

Should probably also rename makeHexString -> toHexString for consistency.

[mikee47]

The notes in http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c5-place-helper-functions-in-the-same-namespace-as-the-class-they-support don't always make sense. In particular, Sming's use of toString would, I feel, be more user-friendly if defined in the global namespace provided the arguments are strongly typed.

For example, in the https://github.com/mikee47/Sming-SSDP/blob/master/src/include/Network/SSDP/Message.h I've defined SSDP::String toString(MessageType type) as per above guidelines, but I now think String toString(SSDP::MessageType type) would make it far more user-friendly.

[slaff]

but I now think String toString(SSDP::MessageType type) would make it far more user-friendly.

I agree with you. Let it be this way.