Quicktype's use of shared_ptrs in C++

[SimonCahill]

What is the reasoning behind using shared_ptr<T> when Quicktype generates C++ classes, especially for primitive types.
It complicates things unnecessarily when simply exchanging primitive data types, such as integers or floating-point numbers.

If I need to cache the variables in my data structures so I can pass them to Quicktype's output, there's no need for using Quicktype at all and I can just as easily design my own header/class which respects my requirements.

[dvdsgl]

We actually don't know C++ very well. This code was a contribution. I believe these types are used to add nullability when a primitive value could be missing.

[SimonCahill]

It would be better to use C++' optional (since C++17) type.

Personally I don't know TypeScript at all, so I don't feel comfortable hacking something in and potentially breaking other things. Is there a possibility to add support for optional without breaking existing code bases?

[dvdsgl]

We can always increment the major version. Would you say optional is categorically better? If so, let’s do it! Should be a straightforward patch.

[SimonCahill]

IMO yes, optional is categorically better. Works with all types (even pointers where needed), has a simple structure:

#include <optional>
#include <string>

namespace MyFoo {
    using std::optional;
    using std::string;

    optional<string> getFooString() {
        if (bar != baz) {
            return std::nullopt; // indicates that the value is missing, AKA null
            // can be simplified to
            return {};
        }
        return "my super awesome foo";
    }

    void doBarStuff() {
        const auto fooString = getFooString();

        if (!fooString.has_value()) { /* it's a null from me. Sorry. */ }
      //^ could also be written
        if (!fooString) { /* it's another null from me! */ }

        // alternative to explicitely checking has_value()
        const auto actualFoo = fooString.value_or("I'm not really the foo!");
    }
}

That's a very simple example of the usage, but I think it's enough to get you going.

Another thing I've noticed: Sometimes raw C-strings are thrown instead of exceptions. You should always throw an exception type.
The STL provides a good number of exceptions, a standard one would be something like std::runtime_error.

[bradphelan]

I had a pull request running with a fix for this and it just disappeared? @dvdsgl You just wrong a few minutes ago that you liked it. Did you accidently delete it?

[dvdsgl]

Yes, the README explains how to do this. Tl;dr set FIXTURE=cplusplus.

[dvdsgl]

You can also do FIXTURE=cplusplus CPUs=1 npm test to make the tests run one-at-a-time (instead of one per core) to make the console output a bit clearer.

[bradphelan]

I've found a problem already. One of the fixtures generates

which means ExtendedBy in the schema is self recursive. How can I detect this and add it to the condition so that in this case the function return false.

    // Returns true if the type can be stored in
    // a stack based optional type. This requires
    // that the type does not require forward declaration.
    isOptionalAsValuePossible(t: Type): boolean {
        if (t.isPrimitive()) return true;
        if (this.isForwardDeclaredType(t)) return false;
        return true;
    }

Is there a better method call that isForwardDeclaredType(t) here?

[dvdsgl]

I don’t know. You’ll have to search for existing cycle detection code—I’m sure we've solved this somewhere before.

[bradphelan]

The rust generator seems to do the right thing on one of my failing test cases. I'll look there.