Value::get_uint should return a Result, not an Option

[alice-i-cecile]

There are several different ways this method can fail.

This return a Result with an enum that implements the Error trait that captures the failure mode and returns it to the end user.

[kamadak]

I see your point. On the other hand, other parts of this library do not raise errors based of the semantics of values.

How about this:

Value::as_uint(&self) -> Option<SomeType>
SomeType::get(&self, index: usize) -> Option<u32>

Then, we can do something like this:

value.as_uint().expect("not unsigned integer")
     .get(3).expect("out of range")

[alice-i-cecile]

On the other hand, other parts of this library do not raise errors based of the semantics of values.

Hmm. Would it make sense to migrate these wholesale?

The core problem that we ran into here was trying to handle the errors more robustly. Your suggestion would definitely help, but it feels a little unidiomatic to not use Result when discussing failure modes.

[kamadak]

By "other parts of this library do not raise errors based of the semantics of values", I meant that this library does not (mostly) care the semantics of the values because it heavily depends on the applications and tags. I did not mean this library returns an Option instead of a Result on an error. Thus, I do not think "migrating these wholesale" makes sense.

So, Value::get_uint is a rare case. If get_uint returns an Option::None, the failure modes cannot be distinguished as you said. However, if it returns a Result::Err on an out-of-bound index, it is inconsistent with slice::get, which returns Option::None.

Maybe this?

Value::as_uint(&self) -> Result<SomeType>
SomeType::get(&self, index: usize) -> Option<u32>

[alice-i-cecile]

Yep, I'd be happy with that! And then link back-and-forth in the docs :)