ISortedSet

[opsb]

I was very happy to find that https://pub.dev/documentation/fast_immutable_collections/latest/fast_immutable_collections/ISet-class.html can be configured to be sorted. In practice though I'm finding it difficult to ensure that a property that is specified as ISet is forced to remain sorted.

@freezed
abstract class TimelineChartData with _$TimelineChartData {
  const TimelineChartData._();

  factory TimelineChartData({
    @required Extent<DateTime> focusedRange,
    @required ISet<GlucoseReading> glucoseReadings,
    @required ISet<InsulinInjection> insulinInjections,
  }) = _TimelineChartData;

  factory TimelineChartData.from({
    @required Extent<DateTime> focusedRange,
    @required TimelineDataBatch batch,
  }) {
    return TimelineChartData(
      focusedRange: focusedRange,
      glucoseReadings: ISet<GlucoseReading>.withConfig(
          batch.glucoseReadings, ConfigSet(sort: true)),
      insulinInjections: ISet<InsulinInjection>.withConfig(
          batch.insulinInjections, ConfigSet(sort: true)),
    );
  }
}

In this example you can see from the factory TimelineChartData.from that I want both glucoseReadings and insulinInjections to be sorted. The problem is that whenever I replace glucoseReadings or insulinInjections I need to remember to configure the ISet to be sorted. By introducing ISortedSet it would allow the compiler to enforce that the set remains sorted.

As an aside thankyou very much for this package, dart deserves a quality immutable package and FIC seems to fill this role very nicely.

[psygo]

I'm not sure if I'm following what you mean, could you please give me some extra help? Is this a bug report or a feature request?

---

As far as I know, FIC currently does memorize your configuration for collections. So you don't need to reconfigure your collection after applying an operation on it and getting a new object. Take the add operation for example:

fast_immutable_collections/test/iset/iset_test.dart

Lines 560 to 575 in 333c4ed

	// 4) With sort
	iset = <int>{}.lock.withConfig(ConfigSet(sort: true)).add(1).add(20).add(3).add(20) as ISet<int>;
	expect(iset.config, ConfigSet(sort: true));
	expect(iset.elementAt(0), 1);
	expect(iset.elementAt(1), 3);
	expect(iset.elementAt(2), 20);
	expect(() => iset.elementAt(-1), throwsRangeError);
	expect(() => iset.elementAt(3), throwsRangeError);
	expect(iset[0], 1);
	expect(iset[1], 3);
	expect(iset[2], 20);
	expect(() => iset[-1], throwsRangeError);
	expect(() => iset[3], throwsRangeError);

So FIC operations do carry the current configuration along. Besides that, you could also set different global configurations with the ImmutableCollection class and the collections' defaultConfig static setters.

---

I believe you actually mean it would be better to, instead of having one single ISet for all of the configurations, having multiple subclasses such that the compiler could also typecheck if the current set is indeed a sorted one, right? If so, I think it's a good idea too, but that's for @marcglasberg to say if it is possible or not given the current implementation constraints and project's design direction. Sorting sets and maps was the last major feature of this package and we had to make quite big changes to make it happen — including ListSet and ListMap was one of those.

At any rate, even though I don't think I fully understood the question or issue, I would ask you to maybe post an extra example without the extra dependency of freezed or any other package. However, of course, if you could clear up some of my implicit questions from above, that might be unnecessary.

[marcglasberg]

@opsb Question: Do you want to accept any ISet (or any Iterable) and then convert them to sorted, or do you want to allow only sorted ISets to be passed into your constructor?

[opsb]

@marcglasberg I want to ask the compiler to enforce that a field always be a sorted set (rather than be a field which can either be sorted or not sorted). I used freezed for the original example because the immutable record style naturally leads to this sort of requirement.

If I use an ISet then this isn't possible, see the following example (using freezed again because I think it gives useful context)

@freezed
abstract class DataBatch with _$DataBatch {
  factory DataBatch({
    @required ISet<int> sortedNumbers,
  }) = _DataBatch;
}

var batch = DataBatch({sortedNumbers: ISet([1,5,3], ConfigSet(sort: true))});
print(batch);
// [1,3,5]

var updatedBatch = batch.copyWith(sortedNumbers: ISet(1, 5, 3));
print(updatedBatch);
// [1,5,3]

Here's the same example but using ISortedSet

@freezed
abstract class DataBatch with _$DataBatch {
  factory DataBatch({
    @required ISortedSet<int> sortedNumbers,
  }) = _DataBatch;
}

var batch = DataBatch({sortedNumbers: SortedISet([1,5,3]});
print(batch);
// [1,3,5]

var updatedBatch = batch.copyWith(sortedNumbers: ISet(1, 5, 3)); // compilation error
var updatedBatch = batch.copyWith(sortedNumbers: ISet.withConfig(1, 5, 3, ConfigSet(sort: false))); // compilation error
var updatedBatch = batch.copyWith(sortedNumbers: ISet.withConfig(1, 5, 3, ConfigSet(sort: true))); // compilation error
var updatedBatch = batch.copyWith(sortedNumbers: ISortedSet(1, 5, 3)); // compiles

The compilation error in each of those cases being that an ISortedSet must be passed not an ISet. With these compiler checks in place it's now impossible for the sortedNumbers field to contain values which haven't been sorted.

[opsb]

I believe you actually mean it would be better to, instead of having one single ISet for all of the configurations, having multiple subclasses such that the compiler could also typecheck if the current set is indeed a sorted one, right?

yes that's exactly right, I'm requesting ISortedSet as a new feature.

[marcglasberg]

Your freezed example makes it harder for me to understand the problem because it's a package I've never used.

Suppose I was to create toISetSorted() and you did this:

class TimelineChartData {
  ISet<GlucoseReading> glucoseReadings;
  ISet<InsulinInjection> insulinInjections;

  TimelineChartData({
    @required Extent<DateTime> focusedRange,
    @required Iterable<GlucoseReading> glucoseReadings,
    @required Iterable<InsulinInjection> insulinInjections,
  }) : glucoseReadings = glucoseReadings.toISetSorted(),
       insulinInjections = insulinInjections.toISetSorted();

Writing .toISetSorted() in the constructor is what you need to ensure that a property is forced to remain an immutable sorted set. In this case, you can pass it any Iterable, and it will always keep it as a sorted for you. Note that if you already pass it a sorted set, it will NOT create a new sorted set from it, but will use the one provided (the .toISetSorted() would simply return this).

However, if you do this:

class TimelineChartData {
  ISet<GlucoseReading> glucoseReadings;
  ISet<InsulinInjection> insulinInjections;

  TimelineChartData({
    @required Extent<DateTime> focusedRange,
    @required ISortedSet<GlucoseReading> glucoseReadings,
    @required ISortedSet<InsulinInjection> insulinInjections,
  });

Then in this case you can only pass it an immutable sorted set.

[opsb]

I wasn't sure if your last example was supposed to be

class TimelineChartData {
  ISortedSet<GlucoseReading> glucoseReadings;
  ISortedSet<InsulinInjection> insulinInjections;

  TimelineChartData({
    @required Extent<DateTime> focusedRange,
    @required ISortedSet<GlucoseReading> glucoseReadings,
    @required ISortedSet<InsulinInjection> insulinInjections,
  });

if so then I think we're on the same page and you can ignore the rest of my post.

I realise that using freezed for my examples has made my motivation for ISortedSet a little opaque so I've used your examples to expand on this.

While constructors may be used to require that an ISet property by created with the sorted requirement, it doesn't do anything to enforce that the field remains that way.

Taking your first example:

class TimelineChartData {
  ISet<GlucoseReading> glucoseReadings;
  ISet<InsulinInjection> insulinInjections;

  TimelineChartData({
    @required Extent<DateTime> focusedRange,
    @required Iterable<GlucoseReading> glucoseReadings,
    @required Iterable<InsulinInjection> insulinInjections,
  }) : glucoseReadings = glucoseReadings.toISetSorted(),
       insulinInjections = insulinInjections.toISetSorted();

If we construct a new object...

var chartData = TimelineChartData(
  focusedRange: focusedRange, 
  glucoseReadings: glucoseReadings.toList(), 
  insulinInjections: insulinInjections.toList(),
);

...then the constructor will convert glucoseReadings and insulinInjections to be sorted sets.

However if we then update the property on the chartData...

chartData.glucoseReadings = ISet(updatedGlucoseReadings);

...then chartData is holding glucoseReadings which aren't sorted. By changing the glucoseReadings property to...

class TimelineChartData {
  ISortedSet<GlucoseReading> glucoseReadings;

...the compiler is able to guarantee that the set is sorted so that the following is a compilation error

chartData.glucoseReadings = ISet(updatedGlucoseReadings);

[marcglasberg]

I'm sorry, what I actually should have written is:

@immutable
class TimelineChartData {
  final ISet<GlucoseReading> glucoseReadings;
  final ISet<InsulinInjection> insulinInjections;

  TimelineChartData({
    @required Extent<DateTime> focusedRange,
    @required Iterable<GlucoseReading> glucoseReadings,
    @required Iterable<InsulinInjection> insulinInjections,
  }) : glucoseReadings = glucoseReadings.withConfig(ConfigSet(sort:true)),
       insulinInjections = insulinInjections.withConfig(ConfigSet(sort:true)),

I am assuming TimelineChartData is immutable, and that glucoseReadings and insulinInjections are final. That's the situation where I think getting any Iterables and doing glucoseReadings.withConfig() works well.

Of course, if glucoseReadings and insulinInjections are NOT final and TimelineChartData is NOT immutable, then yes, you are able to update the proprieties to use unsorted sets, which I agree is bad.

But please note, the sorted/unsorted configuration is only one of the possible configurations for a set. ConfigSet has actually 3 parameters (and maybe more in the future). You can also configure its equality (isDeepEquals parameter) and its hashCode (cacheHashCode parameter). So if I create an ISortedSet and you let users change glucoseReadings directly, it is possible they would still provide an ISortedSet with, say, the wrong equality.

[opsb]

But please note, the sorted/unsorted configuration is only one of the possible configurations for a set.

yeah, this had occurred to me and I've been pondering how best to encode all of those options in the type system. One approach might be to make it easy for developers to subclass ISet and pass the configuration up to the super constructor (or similar approach with an overridden method that returned the config). Another approach might be phantom types but you'd need something like freezed for the ADTs and I suspect would be unfamiliar to Dart developers.

[marcglasberg]

It is already easy to subclass ISet etc. Maybe I should just provide all combinations.

[opsb]

I'm new to Dart so I may well be missing something simple, could you give an example of how to subclass ISet using the ConfigSet options? (or maybe you meant easy in the sense that it could be done easily within the library)

[marcglasberg]

Classes FromISetMixin and FromIterableISetMixin will help you create your own immutable classes based on the ISet. You can then fix, internally, the ConfigSet of your classes.

[opsb]

Thanks for the pointer @marcglasberg. I was seeing some bugs today relating to the sort order being lost in some places. I switched to using the following instead of a straight ISet and poof all the bugs disappeared :)

class IOrderedSet<T>
    with FromISetMixin<T, IOrderedSet<T>>, FromIterableISetMixin<T>
    implements Iterable<T> {
  final ISet<T> _items;

  IOrderedSet([Iterable<T> items = const []])
      : _items = ISet.withConfig(items, ConfigSet(sort: true));

  @override
  ISet<T> get iter => _items;

  @override
  newInstance(ISet<T> iset) {
    return IOrderedSet(iset);
  }

  @override
  T elementAt(int index) => this[index];

  T operator [](int index) {
    return _items[index];
  }
}

Do you see any issues with this implementation?

(edit: I really should have called it ISortedSet 😂)

[marcglasberg]

I don't see any issues with your implementation. Whatever works for you. Please note, your constructor now accepts any Iterable, but if you actually pass it an ISet with the correct configuration it will simply use the same instance you provided. It will not create a new ISet in this case. So, it's a very efficient implementation.

I will leave this issue open because I want to add ISortedSet and other variations in the near future.