optional icon indicating copy to clipboard operation
optional copied to clipboard

`Get` without error / `Unwrap` method useful ?

Open tp opened this issue 5 years ago • 1 comments

Hey,

thanks for this utility :)

Coming from other languages, I wondered that there was not Unwrap which would return the value (or else panic if not set).

My thinking was, that I wrote code like this:

if s.field.Present() {
  doSth(s.field.Unwrap())
}

And then it didn't make sense to me to check the presence again / call Get which could return an error but never would in this case.

Then it occurred to me that I could write:

if field, err := s.field.Get(); err == nil {
  doSth(field)
}

The fact that error would always be errors.New("value not present") and never anything "bad" made me think though if this isn't too complicated to understand for the next reader (who might wonder why we wouldn't log the error etc.)

So basically I have 2 questions:

  • Do you think it's worth to add this succinct if to the docs, or would that be obvious to most Go programmers? My thinking is, is that there is no general way to "handle the error", as mostly I would assume having an optional value not set is totally expected when using this library.
  • Do you think it makes sense to align Get() (or a new method) with the map's behavior of returning value, ok ? That seems a lot less "dramatic" to me, and consumers can still handle cases when they didn't get a value but required one in some branch of their code.

tp avatar Jun 22 '20 07:06 tp

Sorry this has taken me so long to respond to, somehow I did not get notifications for this repo for awhile and I just forgot to check 😬

I think I see your point about:

there is no general way to "handle the error", as mostly I would assume having an optional value not set is totally expected when using this library.

However I dont want to break backwards compatability so this may require a 'new' method to return value, ok instead of value, err

Also, I just released v0.9.0 which adds MustGet which panics if there is no value, which sounds like what you suggested for Unwrap?

markphelps avatar May 04 '21 14:05 markphelps