Classes without default constructor

[Haatschii]

Very interesting library, thank you for publishing it. I started playing around with it and I was wondering if there is any support for classes without default constructor. Considering the following minimal example

class Foo {
	int valueOne;
	int valueTwo;
	int someInvariant;
	
public:
	Foo(int _valueOne, int _valueTwo) :
		valueOne(_valueOne),
		valueTwo(_valueTwo),
		someInvariant(valueOne+valueTwo)
		{}
		
		
	struct glaze {
		using T = Foo;
		static constexpr auto value = glz::object(
			"valueOne", &T::valueOne,
			"valueTwo", &T::valueTwo
		);
	};
};

int main() {
	Foo test{3, 7};

	std::string buffer = glz::write_json(test);
	// Works as expected: {"valueOne":3,"valueTwo":7}

	// auto test2 = glz::read_json<Foo>(test); 
	// Fails to compile because there is no default constructor

	return 0;
}

The deserialization fails to compile as glaze is obviously looking for the default constructor of Foo which is implicitly deleted. However in principle the glaze struct provides sufficient information, to allow it to call the constructor with the values for valueOne and valueTwo in that order, which is, as far as I can recall, the way DAW JSON Link is doing it. Is there already a way to achieve such behavior with glaze, I didn't find? And if not, do you think it is something that would be possible to add to glaze or is it completely contrary to the design?

[stephenberry]

@Haatschii
It's easy to read into classes without default constructors. Just use the syntax that takes in the class by reference:

Foo test{3, 7};
auto parse_error = glz::read_json(test, buffer);

This will parse the buffer into test

[Haatschii]

Thank you for your response. Unfortunately I don't think that this is the right approach, at least for me. One thing is that I don't think this will work if the class in question is nested. E.g.

std::vector<Foo> res;
auto parse_error = glz::read_json(res, R"([{"valueOne":3,"valueTwo":7}, {"valueOne":4,"valueTwo":8}])");

will still fail to compile. But more importantly, I am assuming that there is a reason for the missing default constructor. In my example that was the the someInvariant variable, which should always be the sum of the two values (which is of course a super simplified example). Using the code you suggested someInvariant would usually have an invalid value, as the constructor is not actually called with the correct values.

[stephenberry]

@Haatschii
Ah, I missed your need for the constructor logic and invariant (and nested behavior). I do have a plan and a lot of the code to make this possible, but still have to think through the exact registration syntax.
Right now you could implement your own custom parser for your std::vector<Foo>: custom-serialization.md
But, that isn't a clean approach and would take a lot of work.

I have added your request as an issue #386.

[Haatschii]

Great to hear that you think it is possible to add this kind of functionality! Would love to see that.

I thought about using a custom parser and I might give it a go. But as you said, I too think that would be more of an workaround that a good solution.

[stephenberry]

@Haatschii
So, I've been thinking more about this problem. And, I'm torn about adding the feature.

Let's say we have:

struct MyNonDefaultConstructibleClass
{
   MyNonDefaultConstructibleClass(int x, double y) : x(x), y(y) {
      z = x + y;
   }
   
   int x;
   double y;
   double z;
};

template <>
struct glz::meta<MyNonDefaultConstructibleClass>
{
   using T = MyNonDefaultConstructibleClass;
   static constexpr auto value = object("x", &T::x, "y", &T::y);
};

I can make a constructor based on x and y from this object definition. But, there's a good chance I want to serialize z. So, I need another definition to separate the constructor from the serialization and deserialization.

template <>
struct glz::meta<MyNonDefaultConstructibleClass>
{
   using T = MyNonDefaultConstructibleClass;
   static constexpr auto value = object("x", &T::x, "y", &T::y, "z", &T::z);
   static constexpr auto construct = obj{"x", int{}, "y", double{}};
};

This would be doable, but now my deserialization doesn't match my serialization. Maybe if I have the element z, then I want to deserialize using the value, and if I don't I want to deserialize using construct. But, I find that this becomes more confusing to users. And, if I need to grow a vector then the value of z has to be ignored, or do I do a secondary read to populate this item?

Add to this that the standard library just doesn't play nice with non default constructible classes. To call a resize when just shrinking a vector I need to default construct a class, which is an unnecessary performance overhead and not logical from the JSON side. I can just use default constructible parameters, but if any of these are heap allocated my performance tanks when I'm not even using the value.

I tend to think using non default constructible classes in interfaces is a design flaw:

• It breaks matching serialization and deserialization
• It adds unnecessary performance overhead when using containers
• It forces hidden state from interfaces, making them inflexible
• It bifurcates the logic of instantiation by allocation and overwriting. Overwriting already allocated memory is a huge performance gain, but the constructor obviously won't be called. By making a constructor that behaves differently you're setting your code up to be inefficient and behave differently when overwriting versus construction.

I say this to express that I don't think this is a good feature to add right now. It is easy to add this feature if no invariants need to be computed, but that makes the whole point of this moot.

I think it is best for users to implement custom serializers for this kind of behavior. And, it isn't that much work to accomplish and allows lots of specific logic that will work exactly as the user desires.

[Haatschii]

Hey @stephenberry, thank you for taking the time and your thoughts about this.

Concerning the MyNonDefaultConstructibleClass example I am not sure whether I agree. I think that in practice one would rarely want to serialize the z value, as by construction of the example, it is an invariant of the class, and would be completely redundant, i.e. taking up unnecessary space in any serialization. The only case in which this would make sense is if the calculation of z was so complex that its serialization would give a significant speedup. But then z would be more something like a cache, which could also appear in a default constructable class leading to the same problem of different (de-)serialization paths, depending on the existence of the cache, no?

Anyway, the existence of the invariant was only meant as an example for a class which requires the use of it's constructors. For example having any const or reference members also prevents default initialization of a class without any ambiguity in the deserialization. And more generally, the current design does not (straightforwardly) work for any class which has constrains and/or side effects enforced by its constructors, i.e all classes for which default initialization+overriding all member variables does not work, even if they are default constructible. And even for classes for which this approach works, it could be convenient to allow a syntax such that glaze directly constructs the object using a constructor. In particular if calling the default constructor is not 'free' in terms of performance. A very simple example would be

class LogError {
	int errorCode;
	std::string errorContext;
	
public:
	LogError()  : errorCode(0), errorContext("No error context was given.") { }
	
	LogError(int code, const std::string& msg)  : errorCode(code), errorContext(msg) { }
};

In this example the default constructor contains an allocation which is totally unnecessary if the string is subsequently overwritten by a larger string. Allowing the direct construction via constructor would save this allocation every time.

Thinking about your answer and these example I actually kind of agree that one should not use any such class in interfaces/serialization. And the standard library can indeed be a bit of a hassle with non default constructible classes (albeit in this case myVec.erase(myVec.begin()+newSize, myVec.end()); should do the trick, no?). However changing the design of such classes in an existing project is often unfeasible and one thing I liked about glaze, is that is only minimally invasive to implement for existing classes.

All in all I still think that allowing the use of (one explicit) constructor for the deserialization would be a good feature for your library. But depending on the amount of work this would require I agree that it is probably not a number one priority. And I am probably biased anyway as I do have an existing use case where I would like to have this feature...

[stephenberry]

@Haatschii,

Thanks for your helpful thoughts!

I'll note briefly that if z is a modifiable value by the program, then it very likely may not be reconstructible from x and y, requiring it to be serialized.

I think you're correct that using .erase is a better approach and should avoid the issue! I now plan to change from resize to erase more generally across the library.

I've been playing around with solutions. The main challenge I've run into is that glaze tries to match the C++ layout to a JSON representation, which makes things more logical and cleaner. However, an object of {"x":1, "y":2} doesn't look like the inputs to a C++ constructor, which would more appropriately be [1, 2] (or, a tuple of elements). So, I have to parse a JSON object into a tuple of elements. Add to this the problem that JSON objects can be unordered, so I need to map the input arguments into their appropriate slots in the tuple based on key hashing. This is all totally doable, but it requires mixing low level compile time logic between the constructor definition and the object definition. I will continue to work on a solution. You can look at the construct branch on glaze if you're interested in the current progress.