[QUESTION] About IMap.update

[busslina]

This is my doubt (and maybe you should add it to the method documentation).

In the case that a previous value is present, and the update callback is executed... What if this callback returns the previous value? Will the IMap instance change? Or it will will be the same?

I think it should be the same, but also think you should specify this in the docs.

https://pub.dev/documentation/fast_immutable_collections/latest/fast_immutable_collections/IMap/update.html

Thank you

[marcglasberg]

Currently it returns a new IMap instance:

} else {
      map[key] = updatedValue;
    }

Note I can't compare the updated value with the original value by equality, because equality may return true when objects are different (even if it shouldn't), and also objects in a map may be mutable (even if they shouldn't) so I can't really assume you don't want to replace it if it's equal (you may want to replace it to modify it later).

However, I can optimize it to check if the update callback returns the exact same instance (compare by identity), and in this case yes, return the same IMap instance.

I can specify this in the docs, but that's an implementation detail. I don't think your app should depend on this fact in any way.

[marcglasberg]

Done. It now returns the same IMap instance. This is the new code and documentation:

/// Updates the value for the provided [key].
  ///
  /// 1. If the key is present:
  ///
  /// Invokes [update] with the current value and stores the new value in the
  /// map. However, if [ifRemove] is provided, the updated value will first
  /// be tested with it and, if [ifRemove] returns true, the value will be
  /// removed from the map, instead of updated. Note: If [update] returns
  /// the same INSTANCE as the current value, the original map instance will
  /// be returned, unchanged.
  ///
  /// 2. If the key is NOT present:
  ///
  /// If [ifAbsent] is provided, calls [ifAbsent] and adds the key with the
  /// returned value to the map. If the key is not present and [ifAbsent] is
  /// not provided, return the original map instance, without modification.
  /// Note: If you want [ifAbsent] to throw an error, pass it like
  /// this: `ifAbsent: () => throw ArgumentError();`.
  ///
  /// If you want to get the original value before the update/removal,
  /// you can provide the mutable [previousValue] parameter, which is
  /// of type [Output].
  ///
  @useResult
  IMap<K, V> update(
    K key,
    V Function(V value) update, {
    bool Function(K key, V value)? ifRemove,
    V Function()? ifAbsent,
    Output<V>? previousValue,
  }) {
    // 1. If the key is present:
    if (containsKey(key)) {
      final Map<K, V> map = unlock;

      final originalValue = map[key] as V;
      final updatedValue = update(originalValue);

      previousValue?.save(originalValue);

      // 1.1 Value removed
      if (ifRemove != null && ifRemove(key, updatedValue)) {
        map.remove(key);
        return IMap._unsafeFromMap(map, config: config);
      }
      // 1.2 Value updated
      else {
        map[key] = updatedValue;

        return identical(updatedValue, originalValue)
            ? this
            : IMap._unsafeFromMap(map, config: config);
      }
    }
    //
    // 2. If the key is NOT present:
    else {
      previousValue?.save(null);

      // 2.1 IfAbsent is provided
      if (ifAbsent != null) {
        final updatedValue = ifAbsent();
        final Map<K, V> map = ListMap.fromEntries(
          entries.followedBy([MapEntry(key, updatedValue)]),
          sort: config.sort,
        );
        return IMap._unsafeFromMap(map, config: config);
      }
      //
      // 2.2 IfAbsent NOT provided
      else {
        return this;
      }
    }
  }

[busslina]

Thank you.

Note I can't compare the updated value with the original value by equality, because equality may return true when objects are different (even if it shouldn't), and also objects in a map may be mutable (even if they shouldn't) so I can't really assume you don't want to replace it if it's equal (you may want to replace it to modify it later).

Why don't just letting the programmer to properly handle the equality for their objects and just aborting (by returning the same IMap instance) when both objects are "equals"?

[marcglasberg]

I think that's too complex, making people think about equality, objects being replaced or not, and forcing users to think about if it returns a new instance or the same instance. The update method is a convenience method that's already complex to use, and it's not meant to be useful in all use cases. If users need more specific method they can implement their own, and even add them as extension methods.