-
Notifications
You must be signed in to change notification settings - Fork 321
Add getOrDefault
method to Listing
and Mapping
#1053
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
base: main
Are you sure you want to change the base?
Conversation
I think the method makes sense, but I'm not super convinced about the name.
function getOrDefault(key: K, defaultValue: V): V Some other possible names:
|
@StefMa in |
@HT154 thanks for the explanation! I agree with @bioball that getOrDefault is something different in other languages. This might be confusing. See my original comment 🤦♂️. Still "apply" sounds a bit technical. One questions though, what if default is also not defined? What does it return? 🤔 Update |
If class Foo {
bar: String = "bar"
baz: String
}
value = new Mapping<String, Foo> {
["a"] {
// this amends new Foo
baz = "hello world"
}
}
// new Mapping {
// ["a"] = new Foo {
// bar = "bar"
// baz = "hello world"
// }
// } If no element/value type is present, then I've simplified a little here, but these rules are discussed in a little more detail here: https://pkl-lang.org/main/current/language-reference/index.html#type-defaults |
And in case it helps think about this, here's how things desugar: // rock candy
new Listing<Foo> {
default {
baz = "abc"
}
}
// but default is a lambda
new Listing<Foo> {
default { index ->
baz = "abc"
}
}
// not so sweet
new Listing<Foo> {
default = (index) -> (super.default.apply(index)) {
baz = "abc"
}
} And you might think of class Listing<Element = Dynamic> { // default type argument, not really a thing, this is fake syntax, it's also a lie Any is accepted
default: Function1<Int, Element> = (_) -> new Element {}
}
class Mapping<Key = Any, Value = Dynamic> { // more fake syntax, Any accepted for Value
default: Function1<Key, Value> = (_) -> new Value {}
} |
I think |
This PR adds methods to
Listing
andMapping
that can be used to retrieve members. If the member for the index/key isn't present, it appliesdefault
to the index/key. In both cases, this is essentially sugar forgetOrNull(<index/key>) ?? default.apply(<index/key>)
.I considered adding another method called something like
getOrElse
to these types (as well asList
andMap
). This two-argument method would work likegetOrNull
but in the event of no member being present would return the second argument.I opted not to implement
getOrElse
since the null coalescing operator??
already implements this in a more convenient way:getOrNull(<index/key>) ?? <default value>
. I think the pattern of applyingdefault
should have its own method: for many users it's not always obvious thatdefault
is a lambda, especially when it's defined via the sugared anonymous function object body syntax where the argument may be elided if unused.I'm definitely open to naming suggestions here!
I'm also unsure if
Dynamic
warrants a similar change. I'm currently of the mind thatDynamic.default
should be considered harmful due to #700.