Of those, #1 seems better than #2 and #3. Is there an elegant way to use a
sentinel value of std::string to represent null, e.g., static const
PdxReader::NULL_STRING = <magic happens here>?
Sarge
> On 19 Dec, 2017, at 13:34, Jacob Barrett <jbarr...@pivotal.io> wrote:
>
> Java String is an object and as such a member of type String can be null or
> allocated in the heap. In C++ std::string is a value and as such a member
> of type std::string can never be null and is (usually) stack allocated with
> the contents allocated (usually) on the heap. In C/C++ char* is a decayed
> char[] and a such a member of type char* can be null or a pointer to an
> allocated char[] either on the heap or the stack.
>
> As we move away from char* representations of strings which support null to
> std::string that does not we run into an interesting problem to solve. What
> to do about null strings in a PDX object? The original PdxReader readString
> returned a char*, which could be null or allocated a char[] on the heap and
> returned the pointer (usually, though I found some places where it returned
> the address to a static constant char[] but without const qualifier, bad).
> This version of PdxReader::readString then supports null string values
> coming from Java.
>
> Looking at options for converting to std::string we have the following:
>
> *1) std::string PdxReader::readString(const std::string& fieldName)*
> Most natural for C++11 but does not support null. A null PDX string value
> would be converted into an empty std::string. Upon writing back to PDX the
> null value would be replaced with and empty PDX string value. This could
> impact the other domain objects derived from this same PDX type if they
> treat null and empty strings differently. For example:
> class Foo {
> std::string naturalStringMember;
>
> ~Foo() = default;
>
> void fromData(PdxReader reader) {
> // RVO moved contents
> unnaturalStringMember = readString("naturalStringMember");
> // null value lost
> }
> };
>
>
> *2) std::string* PdxReader::readString(const std::string& fieldName)*
> The subtle difference there is returning raw pointer to a heap allocated
> std::string, which is very very unnatural to C++. The domain object would
> either hold the raw pointer or check for null and copy to a std::string
> value member. In either case they would have to free the allocated
> std::string when done. For example:
> class Foo {
> std::string* unnaturalStringMember;
> std::string naturalStringMember;
>
> ~Foo() {
> if (unnaturalStringMember) {
> delete unnaturalStringMember;
> }
> }
>
> void fromData(PdxReader reader) {
> unnaturalStringMember = readString("unnaturalStringMember");
> if (auto tmp = readString("naturalStringMember")) {
> // move contents
> naturalStringMember = std::move(*naturalStringMember);
> // delete
> delete naturalStringMember;
> } else {
> // null value lost
> }
> }
> };
> *YUCK!!*
>
> *3) std::unique_ptr<std::string> PdxReader::readString(const std::string&
> fieldName)*
> This shares some of the similar issues with unnaturally having a heap
> allocated std::string but solves the ownership ambiguity and deleting
> issues. For example:
> class Foo {
> std::unique_ptr<std::string> unnaturalStringMember;
> std::string naturalStringMember;
>
> ~Foo() = default;
>
> void fromData(PdxReader reader) {
> unnaturalStringMember = readString("unnaturalStringMember");
> if (auto tmp = readString("naturalStringMember")) {
> // move contents and delete
> naturalStringMember = std::move(*tmp);
> } else {
> // null value lost
> }
> }
> };
> *Eh!*
>
> 4) Given that C++ does not allow the same method signature only differing
> in return type we can't have all these methods on the interface unless we
> named them differently as well. For example readString and
> readNullableString. Providing both puts the power in the implementor but
> maybe we should have an opinionated approach.
>
> std::string PdxReader::readString(const std::string& fieldName);
> std::unique_ptr<std::string> PdxReader::readNullableString(const
> std::string& fieldName);
>
> class Foo {
> std::unique_ptr<std::string> nullableStringMember;
> std::string naturalStringMember;
>
> ~Foo() = default;
>
> void fromData(PdxReader reader) {
> nullableStringMember = readNullabletring("nullableStringMember");
> naturalStringMember = readString("uniqueStringMember");
> }
> };
>
>
> 5) Alternatively one could use out parameters, though we have been trying
> to get away from out parameters since they don't match the "functional"
> preference.
> void PdxReader::readString(const std::string& fieldName
> std::string& value);
> void PdxReader::readString(const std::string& fieldName,
> std::string* value);
> void PdxReader::readString(const std::string& fieldName,
> std::unique_ptr<std::string>& value);
>
> class Foo {
> std::string* rawStringMember;
> std::unique_ptr<std::string> uniqueStringMember;
> std::string naturalStringMember;
>
> ~Foo() = default;
>
> void fromData(PdxReader reader) {
> readString("rawStringMember", rawStringMember);
> readString("uniqueStringMember", uniqueStringMember);
> readString("naturalStringMember", naturalStringMember);
> }
> };
>
>
> Does supporting null really buy us anything worth the pain an anguish of
> not looking very C++? My preference is probably towards non-null by
> providing a "nullable" version of the method for backwards compatibility
> (option 4).
>
> Thoughts?
>
> What about arrays of strings? There were supported as a decayed char[][] as
> char**. Defined as char** PdxReader::readStringArray(const std::string&
> fieldName, int32_t& len). Where len was an out param indicating the size of
> the array. The function could return null since Java arrays are objects and
> therefor can be null. Each element of the array could also be null since
> String in Java is nullable. To cleanup one had to iterate the array and
> delete each element if not null then delete the array. Should we consider
> putting this into a std::vector<std::string>. Again we lose the null
> support but maybe it is worth it. Alternatively we could support a nullable
> version as std::unique_ptr<vector<std::unique_ptr<std::string>>>.
>
> More thoughts?
>
> -Jake
