tgockel / json-voorhees

A killer modern C++ library for interacting with JSON.
http://tgockel.github.io/json-voorhees/
Apache License 2.0
128 stars 18 forks source link

Should empty() and size() allow null values? #120

Closed jjberry314 closed 5 years ago

jjberry314 commented 5 years ago

Currently, empty() and size() only allow objects, arrays, or strings, and throw a kind_error for any other type.

Since a null value is by definition empty, and by definition has a size of 0, should empty() and size() do that instead of throwing when the kind is null?

Or do other parts of jsonv rely on the current behaviour?

tgockel commented 5 years ago

This is a really good question that speaks to where the library sits. My current decision was to distinguish between scalars and composite data types fairly strongly, with null being in the scalar value category (which I think it should be). However, I can see an argument for making scalars have these properties. Rephrased: if null is empty() == true and size() == 0, then integers, floats and booleans would be empty() == false and size() == 1. Does that make sense?

jjberry314 commented 5 years ago

Yes, I see what you are saying.

As a compromise, perhaps leave size() as it is, because a null type does not have a size as such, like you say. Its size is "indeterminate" rather than strictly zero.

However, I think having a null type return true for empty() is valid, because "empty" can be taken to mean "does not contain a value".

Certainly, by the Principle of Least Surprise, it surprised me that calling empty() on a value that is a null type throws a kind_error rather than returns true, since to me a null value is by definition empty.

tgockel commented 5 years ago

With that, should all scalar types be non-empty (except null)?

jjberry314 commented 5 years ago

That one is more difficult to answer, as what does "empty" mean in the context of a scalar? Does it mean "has a value"? Or does it mean "has a non-zero value"? Or something else? I think that the current behaviour of throwing a kind_error conveys the concept of "this doesn't really have a meaning for this type".

tgockel commented 5 years ago

Currently, value::empty() means "this value is a composite data type which is or is not empty." Changing null and not the other scalars means that we now think of null as a non-scalar, which doesn't entirely make sense to me. Changing all scalars to non-throwing alters the meaning of value::empty() to "this JSON entity is non-empty" in the vein as JavaScript's truthiness logic, which I prefer to the hybrid approach, although I'm not sold yet.

jjberry314 commented 5 years ago

Hmmmm. I do see your argument. I had no idea that "empty" could be a philosophical concept. :)

For strings, empty has a clear value of "is a string and contains the empty string", and object and array are "is a collection and has no items". So the emptiness refers to what they are holding. I can see that my suggestion of empty() for null is applying the emptiness to the container rather than the value it holds. So I am persuaded by your argument philosophically.

However, on a more practical level, as a programmer if I write if (val.empty()) //... then I'd rather it didn't throw an exception if val is empty (in the sense of "doesn't contain a value"). But then again, I can see your argument that if you do that for null type then you have to consider the numeric types as well.

I suppose one way of looking at it is that null is a quantum state that is neither scalar nor non-scalar nor composite. It is just nothing. And hence is by definition empty.

But, really, I'm completely confused by the whole thing now and my brain hurts. :smile:

jjberry314 commented 5 years ago

Earlier you said

Rephrased: if null is empty() == true and size() == 0, then integers, floats and booleans would be empty() == false and size() == 1. Does that make sense?

Actually, yes, that does make sense. I see what you are saying now. If the type is integer, float, or boolean then by definition it is not empty since it contains a value. I could be persuaded by that argument. Plus it would result in a non-throwing empty() which would actually be advantageous.

jjberry314 commented 5 years ago

The thing that put me down this path in the first place is because I received a bug report of an unhandled exception, and traced it to a kind_error whose what() text showed that it could only have come from size() orempty() Obviously, finding and fixing the call is the correct action (although it is a large mature codebase) but it set me thinking about the whole issue, and why whoever made the call didn't consider the possibility of an exception. I'm a big believer in "The Principle of Least Surprise" and it seems clear to me that the person who made that call didn't consider the possibility there.

Working with legacy code, much of which was written by people who are no longer around, is major suckage sometimes. :)

tgockel commented 5 years ago

As an existential nihilist, emptiness is the only philosophy I truly embrace.

So here's where I'm at right now:

I think this aligns closest with intuition and doesn't have any major negative consequences.

jjberry314 commented 5 years ago

That sounds good to me.

I agree that size() has no real meaning for types that don't have a size.

tgockel commented 5 years ago

Cool -- can you alter PR #121 to reflect the discussion here (and also mark value::empty() as noexcept)?

tgockel commented 5 years ago

Thanks for the contribution!