-
Notifications
You must be signed in to change notification settings - Fork 17.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
proposal: stdlib: provide a standard implementation of the Optional / Maybe generic type #48702
Comments
Can you propose a package and a specific API? Thanks. |
I don't personally have a strong preference for a specific API, but I'm happy to propose a strawman. I have not yet given this serious thought - I'd mostly expected to find an already active thread on the topic, but this should start the ball rolling. Package is an interesting one, it feels too small to need its own package, but I don't see an obvious alternative. package optional
type Value[T any] struct {
value T
ok bool
}
func Ok[T any](value T) Value[T] {
return Value{value: value, ok: true}
}
func (v Value[T]) Empty() bool {
return !v.ok
}
func (v Value[T]) Get() (T, bool) {
return v.value, v.ok
}
func (v Value[T]) Default(fallback T) T {
if v.Empty() {
return fallback
}
return v.value
} Edit: It would probably also be useful to implement some common stdlib interfaces like |
Whatever API is decided on,
I don't think that that's a problem. In terms of actual API, I'd like a On the other end of things, a top-level function, maybe func Of[T any](v T, ok bool) Value[T] {
return Value[T]{v: v, ok: ok}
}
// In another package:
someEnvVariable := optional.Of(os.LookupEnv("SOME_ENV_VARIABLE")).Default("default value").Must() |
I'm honestly unsure how useful this is compared to the typical envVar, ok := os.LookupEnv("SOME_ENV_VARIABLE")
if !ok {
envVar = "default value"
} much more readable than envVar := optional.Of(os.LookupEnv("SOME_ENV_VARIABLE")).Default("default value").Must() I guess what I am getting at is, what are the benefits of |
My rough thinking was that it's better to stick to a minimal API that allows those other functions to be written in packages, than to include them all initially. As long as the underlying type is consistent in the ecosystem and exposes enough information to write the other functions, I don't think there's a big downside to having those be 3rd party functions. |
Somehow related: #11939 (proposal: encoding/json, encoding/xml: support zero values of structs with omitempty) |
I found optionals extremely useful for encoding/decoding: // optional package
type Value[T any] struct {
Value T
Set bool
}
func (v Value[T]) Get() (T, bool) {
return v.Value, v.Set
}
// code usage:
type Error struct {
Type string
Description string
}
type Response struct {
Name optional.Value[string] `json:"name,omitempty"`
Error optional.Value[Error] `json:"error,omitempty"`
}
func parse(data []byte) {
var r Response
json.Unmashal(data, &r)
// Using fields:
r.Name.Value // access to value, will be zero value of not set
if r.Name.Set {
// Explicit check for name to be set.
fmt.Println("Name:", r.Name.Value)
}
// Using methods:
if v, ok := r.Error.Get(); ok {
fmt.Println("Got error:", v.Type)
}
} This will eliminate a whole class of boilerplate, like aws.String() and friends. aws/aws-sdk-go#363 I've personally implemented code generation for optionals and will be very happy not to do so again, it is too ad-hoc and looks like footgun. |
@deanveloper is this in response to the proposal as a whole, or the I agree that for map-like APIs the existing pattern works fine. |
I agree, generally, and, for something like this, most languages would wrap the calls from line to line, but Go's semicolon insertion rules make that look kind of strange: envVar := optional.Of(os.LookupEnv("SOME_ENV_VARIABLE")).
Default("default value").
Must() Compare that with something like Rust: let envvar = env::var_os("SOME_ENV_VARIABLE")
.or(Some("default value"))
.unwrap(); Unfortunately, I get the feeling that this awkwardness will come up a lot more with generics than it did before, as generics enable an entire class of method chaining patterns that were infeasible previously. Edit: Also, like @glenjamin said, I think that for normal functions like data := PredefinedEncodableStruct{
AnOptionalString: optional.Of(SomethingThatUsesTheOKBooleanPattern("arg")).Default("default"),
}
err := enc.Encode(data)
// ... Edit 2: Something else that should be noted: |
An observation on
I agree that there should be some opportunities to realize better APIs with generics around encoding. Record-to-record is one sense of an API direction ... maybe a Swiss Army Record type? I feel like the sense of state-machine-to-state-machine is just fundamentally hard to capture generically ... I can see |
The only "real" examples so far of where Option[T] would be helpful are for encoding and decoding. Does it make sense to put it in package encoding? I think that would help to illustrate that the intent is not to replace the |
This is the problem with the current pointer-based approach as well. Pointer nullability is a side effect of the way pointers work, not the primary purpose, and yet a lot of times pointers are used simply to signify optionality. The is a particular issue because dereferencing can lead to crash bugs, meaning that extra handling has to be done for dealing with incoming data in particular and there's no compile-time safety around it. Optional values provide that safety, as you're really dealing with two completely separate types with no real equiavalance of the automatic dereference of the
I can definitely see the argument for this, but I'm a bit worried that if it winds up being useful elsewhere than it'll be quite awkward for it to be in there, as it can't really be cleanly moved later. |
If it ends up being useful elsewhere, then it should be copied (rather than shared) since it represents a different concept. In my eyes, In terms of optional arguments to functions, I'm a bit torn. Ideally, all zero values should be useful, and functions should see zero values as the "empty" case for an optional argument. However, especially when interacting with other systems (ie databases or other languages), we need a value that is "more zero than zero" (ie NULL for SQL). I'm not sure if a concept like TLDR - I think it might be a better idea to allow each package to define their own |
After playing with go1.18 generics I agree that single Optional is not enough for encoding. because Also, current generics design has bad ergonomics with pointer receiver, and currently the only way to implement it is with newtype (for Settable[T]) and [T, *T]: var v Optional[String, *String] |
It seems like we need more experience with generics, which may turn out to show us that this is not something we can do for all packages in one place. But either way, we need more experience, so we should probably decline this. |
I don't think this is a big problem with Option, right? Option is just a container and doesn't do anything with its contents, unlike the |
This proposal has been added to the active column of the proposals project |
Agree with |
And not just how but whether. |
While I agree with needing experience to determine how/if, I thought that would mean this is put on hold, not declined. Is putting on hold only for shorter term? The process specifies hold when "requires [...] additional information that will not be available for a couple weeks or more" The absolute earliest this would be reconsidered I assume would be post 1.19 release, and more likely post 1.20 release, nearly a year and a half from now, which is a rather long "or more". |
Based on the discussion above, this proposal seems like a likely decline. |
(A bit OT)
Shouldn't it be "postponed until later" instead of outright "decline"? Or "decline" means "decline now" and proposals could be "revived" later, once "stars align" properly - new experience gathered, need is common, etc? |
One thing I will take away from this discussion is that if we do see a variety of third-party func (v Value[T]) Get() (T, bool) {
return v.value, v.ok
}
func Of[T any](v T, ok bool) Value[T] {
return Value[T]{v: v, ok: ok}
} |
@tandr I understand the rejection to mean "not now" indeed. I definitely agree that some form of generic optional type would benefit encoding libraries, if only to provide an alternative to "just use a pointer". But I also agree that offering a standard optional type could encourage everyone to start overusing it enthusiastically, so I can't disagree with a "for now" rejection :) I also agree with @glenjamin that libraries could support arbitrary optional-like types via generic interfaces, as long as there's agreement on the shape or mechanism of those. |
I would also add this this issue is more cross-sectional than just JSON - the same applies to database support. At present you're just about okay if you're using one of the supported types, e.g. https://pkg.go.dev/database/sql#NullString. But then you have sql-specific imports leaking out. But if you're storing any kind of custom type, you need to either use a pointer, or change your type's definition and marshalling to support nullability. |
No change in consensus, so declined. |
I was able to find similar discussions on stdlib generic Slice and Map packages, but I was unable to find anything about
Optional
. If this was just a case of poor searching (or poor indexing), then my apologies - please close this.At the moment, there are two ways to handle optionality - either through the type's zero value (eg
Time
), or by using a pointer type.However, using a pointer type creates a degree of ambiguity. Sometimes a pointer is chosen for mutability reasons, and
nil
is never intended to be present. Likewise sometimes a pointer is chosen only for performance reasons.It has been noted in many places that with the new Go generics system, creating an
Optional[T any]
type to represent a value that may or may not be filled is rather trivial.However, if the creation and use of such types is left up to the ecosystem, it is likely that at least a handful of incompatible implementations may surface - whereas if the stdlib held the main implementation then everyone else could build on top of it as needed.
The text was updated successfully, but these errors were encountered: