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

Ensure that getAll() always returns a mutable list #685

Closed
wants to merge 1 commit into from
Closed

Ensure that getAll() always returns a mutable list #685

wants to merge 1 commit into from

Conversation

aahlenst
Copy link

@aahlenst aahlenst commented Apr 4, 2019

If a box is empty, getAll() returns Collections.emptyList(). This empty list is immutable which can only be noticed at runtime (UnsupportedOperationException is thrown). This is surprising. This pull request changes getAll() in such a way that a mutable list is returned in every case.

@rocboronat
Copy link

Let me downvote this PR. Having immutable objects leads to easier to understand code.

@aahlenst
Copy link
Author

@rocboronat That‘s mostly true but that‘s not what this change is about. The current API makes no promises regarding immutability. It promises a java.util.List, which is a mutable container. And in most of the cases, you actually get a mutable list. But if the box is empty, you suddenly get an immutable list. This is bonkers because it turns a compile-time error into a runtime problem.

In my opinion, It‘d be perfectly fine if ObjectBox consistently returned immutable collections. But this would require a change to the public API and the introduction of a proper immutable list type without any mutators. Otherwise the compiler cannot help.

@greenrobot-team greenrobot-team self-assigned this Jun 3, 2019
@greenrobot-team greenrobot-team added this to the 2.4.0 milestone Oct 7, 2019
@greenrobot-team
Copy link
Member

greenrobot-team commented Oct 7, 2019

Thanks for reporting. Starting with 2.4.0-RC Box.getAll() always returns a mutable list.

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.

None yet

3 participants