Skip to content

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

HT154
Copy link
Contributor

@HT154 HT154 commented Apr 25, 2025

This PR adds methods to Listing and Mapping that can be used to retrieve members. If the member for the index/key isn't present, it applies default to the index/key. In both cases, this is essentially sugar for getOrNull(<index/key>) ?? default.apply(<index/key>).

I considered adding another method called something like getOrElse to these types (as well as List and Map). This two-argument method would work like getOrNull 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 applying default should have its own method: for many users it's not always obvious that default 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 that Dynamic.default should be considered harmful due to #700.

@bioball
Copy link
Member

bioball commented May 1, 2025

I think the method makes sense, but I'm not super convinced about the name.

getOrDefault has a different semantic in other languages (like Java). I'd expect the signature to look like:

function getOrDefault(key: K, defaultValue: V): V

Some other possible names:

  • getOrApplyDefault
  • getDefaulting

CC @holzensp @stackoverflow

@HT154
Copy link
Contributor Author

HT154 commented May 2, 2025

@StefMa in Listing<Element> and Mapping<Key, Value>, the default isn't a concrete value, it's a Function1<Int, Element> or Function1<Key, Value>, respectively. The lambda syntax is often sugared and the arg omitted when unused so it's easy to miss this! In cases where the index/key is not used applying the default and returning the result is "returning the default value".

@StefMa
Copy link
Contributor

StefMa commented May 2, 2025

@HT154 thanks for the explanation!
I already deleted my comment. Was too early in the morning 🫣

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. gefOrUseDefault 🤔 getOrUse[Listings/Mappings]Default.

One questions though, what if default is also not defined? What does it return? 🤔
If it's crashing "as the element is also not available". We could go with getOrDefaultOrNull 👀...

Update
Maybe just getOrDefaultValue
If you see such a function you might start thinking "What is the default value for this list?", if not defined. Search for this, find the default lambda and so on... 🙃

@HT154
Copy link
Contributor Author

HT154 commented May 2, 2025

If default isn't explicitly defined on the Listing/Mapping, the default lambda returns the default value of element/value type. That's why this works:

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 new Dynamic {} is used instead.

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

@HT154
Copy link
Contributor Author

HT154 commented May 2, 2025

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 Listing and Mapping as defined something like this:

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 {}
}

@stackoverflow
Copy link
Contributor

I think getOrDefault is a good name. In Java, the signature is getOrDefault(index, default). So it's already clear by the signature that the Pkl version is doing something different, and it's picking the default somewhere else.
The other suggestions still have default in the name, which won't help with the confusion, so may as well use the better name.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants