thriftrw / thriftrw-go

A Thrift encoding code generator and library for Go
MIT License
100 stars 53 forks source link

Is there a reason why we cannot have a getter for array items? #552

Open kanstantsin-chernik opened 2 years ago

kanstantsin-chernik commented 2 years ago

I like the idea of safe autogenerated getters. But every time I have to check the length of an array I am confused as this job is so routine and dumb that is a perfect match for a autogenerated wrapper. Consider example

if len(val.GetArrayField()) > 0 {
  val.GetArrayfield()[0].GetSomeNestedField()
}

can become

val.GetArrayfieldItem(0).GetSomeNestedField()

All we need is to generate array getter like this:

func (Value *v) GetArrayfieldItem(int32 index) *Field
if v != nil && index >= 0 && len(val.ArrayField) > index {
  return val.ArrayField[index]
}
return nil

Is there any fundamental reasons why we cannot have array getters like this or nobody just got to do it?

kanstantsin-chernik commented 2 years ago

CC: @shirchen

shirchen commented 2 years ago

Hey, sorry for delay in responding. I like this feature and have benefited from python's .get("foo", {}).get('bar') default, so i think tradeoff of this feature is whether the bugs that will happen due to silent behavior of your proposal are worth the syntactic sugar.

My question is what we need to return if a field isn't there. Do you return nil? What if someone is doing v := val.GetFoo(1).GetBar(2). Then is nil coming from the Bar not have 2 indexes or from Foo? Or is Bar field only size of 2. Then, I can see that this may make it tough from a developer perspective on how to account for this case may have a v == nil assuming only one of the scenarios i mentioned above, so automatic getters may introduce silent bugs. Then it's a question of whether a user may be better off not using them in the first place.

So I can go both ways on this proposal, and philosophically, I think its sometimes easier to keep things simple. Let me know what you think.

kanstantsin-chernik commented 2 years ago

Hey, thank you for getting back to me. I must admit, my use cases are just wrong... I tried to make things like v.GetA().GetB()[0].GetC() safer while the problem is I shouldn't have to deal with such a nested object in the first place. However, the reality is, many folk have the same ugly objects to work with... If we look at the current chains without any arrays like v.GetA().GetB().GetC(), I would argue that we don't really care where the nil shows up. What we care is the end result