search

amazon-ion / ion-go

A Go implementation of Amazon Ion.
https://amazon-ion.github.io/ion-docs/
Apache License 2.0
175 stars 31 forks source link

Inconsistency on the reader interface when returning values. #152

Closed byronlin13 closed 4 years ago

byronlin13 commented 4 years ago

Currently on reader API, some values that return *type so they return a nil if the Ion value is null. e.g., DecimalValue

However most of them return an err with the expectation that you will have handled null separately through a combination of IsNull() and Type(). (This isn't documented yet, by the way. See IntValue() for an example -- the doc comment doesn't explain what to expect if the value is null.)

This ticket is a discussion on whether we should return *type for all values.

byronlin13 commented 4 years ago

After discussion with @zslayton and @therapon , it has been decided to update the value APIs to return (*type, error) to handle Ion values that are null by returning nil

Proposed changes on Reader API:

BoolValue() (bool, error) -> BoolValue() (*bool, error)
IntValue() (int, error) ->  IntValue() (*int, error) 
Int64Value() (int64, error) -> Int64Value() (*int64, error)
Uint64Value() (uint64, error) -> Uint64Value() (*uint64, error)
BigIntValue() (*big.Int, error) -> no change
FloatValue() (float64, error) -> FloatValue() (*float64, error)
DecimalValue() (*Decimal, error) -> no change
TimestampValue() (Timestamp, error) -> TimestampValue() (*Timestamp, error)
StringValue() (*string, error) -> no change
ByteValue() ([]byte, error) -> no change
zslayton commented 4 years ago

Lgtm 👍 

Int64Value() (int64, error) -> Int64Value() (*int64, error)
Uint64Value() (uint64, error) -> Uint64Value() (*uint64, error)

This reminds me: we still need to discuss #102.