const UnmodifiableSetView.empty() constructor

[simon-void]

It would be really nice to have a const Unmodifiable{List/Map/Set}View.empty() constructor or Empty{List/Map/Set} classes with const constructor because only const values can be used as default arguments for optional parameters!
The is a const Iterable.empty() constructor in Dart:core, but the same for List/Map/Set is missing and the List/Map/Set.from{Iterable}(Iterable i) constructors don't help us because they aren't const.

[nex3]

UnmodifiableListView.empty() and UnmodifiableMapView.empty() seem redundant with const [] and const {}, respectively. I could see an argument for const UnmodifiableSetView.empty(), though.

[simon-void]

wow, i didn't know that you could write const [] and const {} :)
I agree, we're down to const UnmodifiableSetView.empty()

[simon-void]

i could add this constructor :)
how does this work, do you give me contributor rights in advance or do i fork your project, do the change on my version an request a merge? Or do i pull your project, do a commit locally, and pushing it ends up in pull requests?
I haven't contributed to open-source projects before so i'm not sure how to do this.

[nex3]

Normally you fork this repository, push the change to your fork, and then send a pull request here.

[simon-void]

ok, so i forked the repository and added a const constructor to UnmodifiableSetView
const UnmodifiableSetView.empty() : this( const _EmptyUnmodifiableSet());
Too bad that this results in a compiletime error message: Constant constructor cannot be declared for a class with a mixin
So my original approach doesn't seem to work.

Instead i could make my newly implemented class _EmptyUnmodifiableSet public. This class has a const constructor and implements the wanted functionality of an unmodifiable, empty set and has the additional advantage that remove-operations don't throw exceptions (because remove-operations don't change anything on empty lists). Only add() and addAll() throw an Error. If you aggree, i should probably move the class to a different dart-file because unmodifiable_wrappers.dart doesn't sound appropriate for a class that is unmodifiable but doesn't wrap anything.
What do you think?
My code is here.

[nex3]

You can avoid that error by using a redirecting factory constructor, which is a little-known corner of the language useful for exactly this case. Basically you write

const factory UnmodifiableSetView.empty() = _EmptyUnmodifiableSet;

and that makes const UnmodifiableSetView.empty() return const _EmptyUnmodifiableSet() without needing to make the latter public.

[simon-void]

ok, that work if i also change
class _EmptyUnmodifiableSet<E> extends IterableBase<E> implements Set<E>
to
class _EmptyUnmodifiableSet<E> extends IterableBase<E> implements UnmodifiableSetView<E>
because _EmptyUnmodifiableSet isn't assignable to UnmodifiableSetView otherwise.
This still means that we have cases where unmodifiableSetView.remove(o) will throw an error and some where that isn't the case. Of course i could also reduce the functionality of _EmptyUnmodifiableSet by also throwing errrors on remove, but that hurts a little.
What do you think? (I updated the code.)

[nex3]

_EmptyUnmodifiableSet should definitely throw errors on all modification operations, just like UnmodifiableSetView does. It should be behaviorally identical to new UnmodifiableSetView(new Set()).

[simon-void]

but look at the definition of Set.remove, semantically trying to remove an element from an empty list simply returns false without modifying the list. So the set is still unmodified and empty ;-)
It's only an issue when you use it in the context of UnmodifiableSetView. The alternative would be to make _EmptyUnmodifiableSet public. So if we need a const empty set, we would write
const EmptyUnmodifiableSet() instead of const UnmodifiableSetView.empty().

advantage:

• a more powerful/versatile implementation of a const empty set because even the remove-operations (remove, removeAll, removeWhere, retainWhere) work as defined by Set

disadvantage:

• one more class in the library

It's your choice.

[nex3]

The term "unmodifiable" has a very specific meaning in Dart, and especially in the context of this package. It means that all mutation operations throw—there's no exception based on whether the operation would actually change the state of a modifiable version of the class. I would expect any class with "unmodifiable" in the name to abide by those rules, whether it was attached to UnmodifiableSetView itself or not.

[simon-void]

fair enough, the programmer in me understands (while the grammar nazi in me wants to point out that this very specific meaning of "unmodifiable" is different from the general meaning of "unmodifiable" :P . Yes, programming languages are less ambiguous than natural language, i understand).
Luckily the programmer in me is responsible for writing code so i will adapt my implementation :)
(After all nobody would expect to find a class called UnchangeableEmptySetin this package :D )

[simon-void]

ok, i updated my code and added tests for UnmodifiableSetView.empty()

  Set aSet = new Set();
  testUnmodifiableSet(aSet, new UnmodifiableSetView(aSet), "empty");
  aSet = new Set();
  testUnmodifiableSet(aSet, const UnmodifiableSetView.empty(), "const empty");

Is my next step the pull request?

[nex3]

Yep!