Skip to content
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

Builder defaults for collections are inconsistent with output record #122

Open
fprochazka opened this issue Jul 19, 2022 · 3 comments
Open
Labels
enhancement New feature or request PR welcome A PR submitted for this issue would be welcome

Comments

@fprochazka
Copy link

Hi,
awesome library! Love it! I just have a small suggestion...

If I have a record with any collection

@MyRecordBuilder // with useImmutableCollections = true, interpretNotNulls = true
public record Costs(
    @Nullable BigDecimal shipping,
    Map<String, Object> extraData
) { }
  • the default values when the builder initializes for all fields is null
  • calling CostsBuilder.builder().build().extraData() builds instance of Costs that has extraData initialized with empty Map.of() and does not return null
    • which means CostsBuilder.builder().build().extraData().get("value") will not throw NPE
  • calling CostsBuilder.builder().extraData() returns the default null
    • which means CostsBuilder.builder().extraData().get("value") will throw NPE

I'm proposing that since you're adding special handling of collections and always initializing them, they should also be initialized to empty collections when the empty builder is created, so that the second case will also not throw an NPE

@Randgalt
Copy link
Owner

Could these be combined into a single PR? Maybe with a new option to avoid breaking existing code.

@fprochazka
Copy link
Author

@Randgalt I've created two issues because I expected others to have a different opinion and you might be willing to accept only one part of the proposal, but both of the issues are related in my mind.

@Randgalt Randgalt added enhancement New feature or request PR welcome A PR submitted for this issue would be welcome labels Sep 8, 2022
@hofiisek
Copy link
Contributor

Isn't this one solved by this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request PR welcome A PR submitted for this issue would be welcome
Projects
None yet
Development

No branches or pull requests

3 participants