[PR #1053] [MERGED] Add getOrDefault method to Listing and Mapping #867

Closed
opened 2025-12-30 01:27:24 +01:00 by adam · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/apple/pkl/pull/1053
Author: @HT154
Created: 4/26/2025
Status: Merged
Merged: 7/23/2025
Merged by: @bioball

Base: mainHead: getordefault


📝 Commits (3)

  • d845b3b Add getOrDefault method to Listing and Mapping
  • f55a6ff address review feedback
  • 4c3d26e Apply suggestions from code review

📊 Changes

7 files changed (+65 additions, -2 deletions)

View changed files

📝 pkl-core/src/main/java/org/pkl/core/stdlib/base/ListingNodes.java (+16 -1)
📝 pkl-core/src/main/java/org/pkl/core/stdlib/base/MappingNodes.java (+18 -1)
📝 pkl-core/src/test/files/LanguageSnippetTests/input/listings/default.pkl (+4 -0)
📝 pkl-core/src/test/files/LanguageSnippetTests/input/mappings/default.pkl (+4 -0)
📝 pkl-core/src/test/files/LanguageSnippetTests/output/listings/default.pcf (+4 -0)
📝 pkl-core/src/test/files/LanguageSnippetTests/output/mappings/default.pcf (+4 -0)
📝 stdlib/base.pkl (+15 -0)

📄 Description

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.


🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/apple/pkl/pull/1053 **Author:** [@HT154](https://github.com/HT154) **Created:** 4/26/2025 **Status:** ✅ Merged **Merged:** 7/23/2025 **Merged by:** [@bioball](https://github.com/bioball) **Base:** `main` ← **Head:** `getordefault` --- ### 📝 Commits (3) - [`d845b3b`](https://github.com/apple/pkl/commit/d845b3b155e11f35ce5a14fc5d4358eddf30eac1) Add getOrDefault method to Listing and Mapping - [`f55a6ff`](https://github.com/apple/pkl/commit/f55a6ff256e3529d7f2360f97d2aa07f433a5708) address review feedback - [`4c3d26e`](https://github.com/apple/pkl/commit/4c3d26e42ced5341aa21c1ebb80704044656a4b9) Apply suggestions from code review ### 📊 Changes **7 files changed** (+65 additions, -2 deletions) <details> <summary>View changed files</summary> 📝 `pkl-core/src/main/java/org/pkl/core/stdlib/base/ListingNodes.java` (+16 -1) 📝 `pkl-core/src/main/java/org/pkl/core/stdlib/base/MappingNodes.java` (+18 -1) 📝 `pkl-core/src/test/files/LanguageSnippetTests/input/listings/default.pkl` (+4 -0) 📝 `pkl-core/src/test/files/LanguageSnippetTests/input/mappings/default.pkl` (+4 -0) 📝 `pkl-core/src/test/files/LanguageSnippetTests/output/listings/default.pcf` (+4 -0) 📝 `pkl-core/src/test/files/LanguageSnippetTests/output/mappings/default.pcf` (+4 -0) 📝 `stdlib/base.pkl` (+15 -0) </details> ### 📄 Description 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. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
adam added the pull-request label 2025-12-30 01:27:24 +01:00
adam closed this issue 2025-12-30 01:27:24 +01:00
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: starred/pkl#867