On Thu, Nov 09, 2023 at 08:01:48PM +0100, Victor Toso wrote:
> On Thu, Nov 09, 2023 at 09:34:56AM -0800, Andrea Bolognani wrote:
> > We should call this Null instead of IsNull, and also make it a
> > pointer similarly to what I just suggested for union branches
> > that don't have additional data attached to them.
>
> I don't have a strong argument here against what you suggest, I
> just think that the usage of bool is simpler.
>
> The test I have here [0] would have code changing to something
> like:
>
> When is null:
>
> - val := &StrOrNull{IsNull: true}
> + myNull := JSONNull{}
> + val := &StrOrNull{Null: &myNull}
>
> So you need a instance to get a pointer.
>
> When is absent (no fields set), no changes as both bool defaults
> to false and pointer to nil.
>
> The code handling this would change from:
>
> - if u.IsNull {
> + if u.Null != nil {
> ...
> }
>
> We don't have the same issues as Union, under the hood we Marshal
> to/Unmarshal from "null" and that would not change.
>
> [0]
> https://gitlab.com/victortoso/qapi-go/-/blob/main/test/type_or_null_test.go
>
> I can change this in the next iteration.
No, leave the type alone. But I still think the name should probably
be Null instead of IsNull.
--
Andrea Bolognani / Red Hat / Virtualization
