pkl icon indicating copy to clipboard operation
pkl copied to clipboard

Add `getOrDefault` method to `Listing` and `Mapping`

Open HT154 opened this issue 8 months ago • 6 comments

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.

HT154 avatar Apr 25 '25 23:04 HT154

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

bioball avatar May 01 '25 20:05 bioball

@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".

HT154 avatar May 02 '25 03:05 HT154

@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... 🙃

StefMa avatar May 02 '25 03:05 StefMa

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 avatar May 02 '25 04:05 HT154

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

HT154 avatar May 02 '25 04:05 HT154

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.

stackoverflow avatar May 02 '25 08:05 stackoverflow

Amy idea what's up with CI here? Looks like an infra failure downloading JDKs.

HT154 avatar Jul 22 '25 15:07 HT154

Amy idea what's up with CI here? Looks like an infra failure downloading JDKs.

Might be https://github.com/foojayio/discoapi/issues/124? 🤔 Which is used via the foojay gradle plugin (settings.gradle.kts).

StefMa avatar Jul 22 '25 16:07 StefMa