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

Collection setter signature depends on addSingleItemCollectionBuilders option #117

Open
freelon opened this issue May 22, 2022 · 8 comments

Comments

@freelon
Copy link
Contributor

freelon commented May 22, 2022

@RecordBuilder
public record SomeRecord(List<Integer> l) {
}

generates public SomeRecordBuilder l(List<Integer> l) {

while adding

@RecordBuilder.Options(
    addSingleItemCollectionBuilders = true
)

changes it to public SomeRecordBuilder l(Collection<? extends Integer> l) {.

@Randgalt
Copy link
Owner

Damn - this is hard to fix without breakage. It needs some thought.

@Randgalt
Copy link
Owner

I wonder if we just decide to change the other one to Collection - I don't think it will cause much disruption.

@freelon
Copy link
Contributor Author

freelon commented May 23, 2022

I wouldn't think so, either.
A check would be needed whether the parameter is in fact the specified type, e.g. List, and if so, just store it, otherwise create a copy of it. The question would be what type the copy should be of, though. An immutable list or just ArrayList?

@Randgalt
Copy link
Owner

@freelon I lost track of this. Does the PR I just merged assert the change we discussed?

@freelon
Copy link
Contributor Author

freelon commented Jun 12, 2022

@Randgalt I'm forgot myself, but I don't think so. I'll check later, but I can provide another MR for it.

@freelon
Copy link
Contributor Author

freelon commented Jun 20, 2022

I was thinking about this again, and I think it's a bit counterintuitive to have a parameter of type Collection<T> in the setter. It would just be a special case for Set<T> and List<T> and as I mentioned above, the question would be of which type an internally allocated list or set should be, if the parameter was of some other collection type.
I did a quick search on GitHub for usages of the setter-methods where a non-list/non-set collection was used and I didn't find any (generally I didn't find a usage of the addSingleItemCollectionBuilders flag outside this project). So I think the breaking change from Collection<T> to List/Set<T> for that parameter would be fine.

@Randgalt
Copy link
Owner

My concern is does this limit utility? List is a type of Collection and people can create custom classes that implement Collection but not List/Set. Thoughts?

@freelon
Copy link
Contributor Author

freelon commented Jun 22, 2022

Well in that case they could just define a property in the record as type Collection<T>, then they would have the setter with parameter type Collection. Or am I misunderstanding what you mean?

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

No branches or pull requests

2 participants