'undefined' return when an optional field does not exist about gpb HOT 5 CLOSED

It would be relatively easy to add this kind of type-specific defaults, but just adding it straight off would also mean that it would be impossible to see if an optional message is not present. The (C++, Java, Python) classes generated from google's protobuf all have a has_xyz()method for each field, so there it is possible to tell. For gpb, one corresponding approach could be to add similar has_xyz fields, but I would not like that at all: doubling the record/map size, decreasing performance. I have not found a way that both looks nice, makes it possible to see if optional fields are present or not, and does not decrease performance, but ideas are welcome, of course, but until then, I'm not about to add type-specific defaults.

from gpb.

The ruby gem I have used for protobuf related things handles this by creating two methods with different semantics. So for optional uint32 age it would generate two methods like this:

decoded.name # => 0
decoded.name! # => nil

The method name with the bang appended gives you the semantics where you can see the nil and the method without the bang follows the protobuf specification.

Not sure if that is helpful or not, but after using that gem in production for several years I still find this to be a good tradeoff.

from gpb.

Thanks @mmmries, this is good food for thought. Continuing along those lines, but for gpb, there could be some alternatives:

• make two record fields for every optional proto message field, one which has the value undefined | value() as it is in gpb today, and the other which has the default (or type default) value. But I'm not very keen on adding extra fields to the record, mostly for performance reasons, so another idea is to:
• generate a function default_<Msg>_<field>() for for every such optional field, or perhaps to:
• generate a function defaultify/1 which would take a message, and set the default value for every non-set optional field (recursively), so one could call X:defaultify(X:decode_msg(Bin)).

@yizhenfei, could you describe your use case a bit, for me to get a better understanding? If a use case is only to simply retrieve a default value specified in the proto file, then the generated introspection function might perhaps(?) provide what you need already:

If you have a proto file like this:

message m1 {
  optional uint32 f2 = 2 [default=333];
}

then gpb will generate a function like this:

find_msg_def(m1) ->
    [#field{name = f2, fnum = 2, rnum = 2, type = uint32,
            occurrence = optional, opts = [{default, 333}]}];

from gpb.

@yizhenfei :

1. what should be the default value for an optional ENUM type ?
2. what should be the default value for an optional field that is also a protocol buffer ?
3. for performance reasons, it might make sense to have undefined's. If you have a message with 200 optional fields, if for example you write them to a database and read them back, having undefined for all of them would save a lot of space : if we typecast undefined to each field's underlying type (assuming we find a way to do that - see my previous two questions !) then we will write in the db some default values - so when we read them back we WILL interpret them as valid fields and we WILL try to encode them into a protocol buffer. So, for encoding, "optional uint32 bla = 2;" encoded as 0 takes some bytes (or thereabout) whereas encoded as undefined takes close to 0 bits (since the protocol buffer encoder has to encode has_bla = NO).

Please don't typecast undefined's - pretty please, with a cherry on top ?

from gpb.

@ionuthristodorescu, Good points. I'm going to close this as won't fix.

At the moment, it looks like the need for getting type-specific default values does not warrant the effort, but should things change, it is always possible to add this later on, for example by generating a defaultify(Msg) function, or perhaps similar. In any case, the current behaviour of decoding unset optional fields to undefined will not be changed.

from gpb.