5 ms·
type Optional[T any] struct { value *T } Don't use a pointer - it's bad for performance. func MakeOptional[T any](value *T) Optional[T] { Use `New` i
by Mawr 5y ago
type Optional[T any] struct {
value *T
}
Don't use a pointer - it's bad for performance.
func MakeOptional[T any](value *T) Optional[T] {
Use `New` instead - it's the idiomatic name for a constructor in Go. Drop the `Optional` part - the module name suffices - the caller will see: `opt.New`. Personally I just call the module `optional`, I think it's clearer: `optional.New`.
func (o* Optional[T]) Unwrap() (T, error) {
1. `Unwrap` reminds me of Rust's .Unwrap, which panics. Seems confusing.
2. There's no error here, the situation is equivalent to a missing key in a map - it's enough to return a bool.
Here's my implementation so far: https://gist.github.com/Mawr-BF2/0a60da26f66b82ee87b98b03336e1f84 https://gist.github.com/Mawr-BF2/0a60da26f66b82ee87b98b03336.... Only thing I'm not sure about is whether the JSON serialization methods are necessary.
- wtfishackernews 5y agoFor json marshalling, I would defer to the underlying type's marshalling if it's present, and return `null` otherwise.
- shhsshs 5y agoI think this is appropriate in some cases but not others. For example how does the JSON value distinguish between `Some(null)` and `None`?
- dialogbox 5y agoIMO using Optional type means the inner value must be not null. And if it's Some(null), it should mean exactly same as None.
- uryga 5y agodistinguishing `Some(null)` and `None` is often considered a feature of Optional ;) to use a tired example: when getting a value out of a map via some `myMap.get(key)`, you may want to distinguish "not present" = `None` and "present, with value null" = `Some(null)` the right solution is to just not have nulls in the first place, then there's no problem ;)
- alophawen 5y ago(Assuming we are discussing the Option type from rust) Some(null) is not a valid result. The whole point of the Option type is to let you know: a) we got a result: Some(value) b) there is no result: None
- andrewjf 5y agoIt's trivial to store a null pointer in an Option::Some(). struct Foo; fn main() { let option_with_null: Option<*const Foo> = Some(std::ptr::null()); dbg!(option_with_null); dbg!(option_with_null.is_none()); } Output: [src/main.rs:6] option_with_null = Some( 0x0000000000000000, ) [src/main.rs:7] option_with_null.is_none() = false https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=6b87832bf48f4115ae44504e497287c4 https://play.rust-lang.org/?version=stable&mode=debug&editio...
- frenchie4111 5y agoI love it. I will update the library to match some of your suggestions. I agree with peer comment that we should likely defer to the original type for JSON serialization.
- morelisp 5y agoWhether a pointer is bad for performance in this case seems hard to tell a priori. There are also performance advantages if Optional packs nicely.
- frenchie4111 5y agoAgreed on performance being in the air. Tbh, I like the non-pointer option because using a pointer was causing the implementation to feel a bit weird. This cleans up a bit of the handling that was annoying (for example in the old implementation Make(&"something") didn't work because you can't reference a constant without first making a variable.
- Mawr 5y agoIt is just an assumption on my part, I figure that the cost of pointer chasing is generally going to outweigh any disadvantages. It can also help enable optimizations, for example slices and maps that do not contain pointers do not get scanned by the GC ([1]). [1]: https://github.com/golang/go/commit/85e7bee19f9f26dfca414b1e9054e429c448b14f https://github.com/golang/go/commit/85e7bee19f9f26dfca414b1e...
- stingraycharles 5y agoI fully agree with your approach. The important thing is that the user now has the choice to use a pointer or not: they can always use a pointer to the optional if they want to use pointers. Pointers to pointers are generally something you try to avoid if possible.
- morelisp 5y ago> the user now has the choice to use a pointer or not But any benefits of the packing are gone whether I make a pointer or not. And either way, no nested pointers are involved. We’re not taking pointers to options in either case.
- the-smug-one 5y ago> Don't use a pointer - it's bad for performance. That means you must have Optional[*sync.Mutex]. Then what's the point?
- kesslern 5y ago> Use `New` instead - it's the idiomatic name for a constructor in Go. Where do you get this from? I've tried unsuccessfully to find such naming/design guidelines.
- dbaggerman 5y agoIt's noted in the Effective Go guide: https://go.dev/doc/effective_go#package-names https://go.dev/doc/effective_go#package-names
- baby 5y agoWondering how a result type would be implemented