Suggestion to add map method and...

[iamarnas]

Hi :wave:

I would recommend to add map methods if the user generates everything @WithWrap(). For now, I am forced every time to add this manually. Map method is useful by copyWith() method.

//...
  R map<R extends Object?>({
    required R Function() loading,
    required R Function(ResultError<T, E> error) error,
    required R Function(ResultData<T, E> data) data,
  }) {
    final result = this;
    if (result is ResultLoading<T, E>) {
      return loading();
    } else if (result is ResultError<T, E>) {
      return error(result);
    } else if (result is ResultData<T, E>) {
      return data(result);
    } else {
      throw AssertionError();
    }
  }

  R mapOrElse<R extends Object?>({
    R Function()? loading,
    R Function(ResultError<T, E> error)? error,
    R Function(ResultData<T, E> data)? data,
    required R Function(Result<T, E> result) orElse,
  }) {
    final result = this;
    if (result is ResultLoading<T, E>) {
      return loading != null ? loading() : orElse(result);
    } else if (result is ResultError<T, E>) {
      return error != null ? error(result) : orElse(result);
    } else if (result is ResultData<T, E>) {
      return data != null ? data(result) : orElse(result);
    } else {
      return orElse(result);
    }
  }
//...

Instead of functions, I think the getters are better for cleaner code syntax. Example:

  bool get isLoading => this is ResultLoading<T>;
  bool get isError => this is ResultError<T>;
  bool get isData => this is ResultData<T>;

  ResultLoading<T> get asLoading => this as ResultLoading<T>;
  ResultError<T> get asError => this as ResultError<T>;
  ResultData<T> get asData => this as ResultData<T>;

  ResultLoading<T>? get asLoadingOrNull {
    final result = this;
    return result is ResultLoading<T> ? result : null;
  }

  ResultError<T>? get asErrorOrNull {
    final result = this;
    return result is ResultError<T> ? result : null;
  }

  ResultData<T>? get asDataOrNull {
    final result = this;
    return result is ResultData<T> ? result : null;
  }

and then you can use it.

final value = data.asData.value;
// Instead
final value = data.asData().value; 

Of course, it's a taste thing. It's just my opinion.

And Dart Sealed Class Generator needs updated analyzer to 1.7.0 => 2.0.0 it conflicts with other packages for example with most one popular Freezed or json_serializable. It would make easier life to config pubspec.yaml file for beginners :smiley:

And factory constructors prefer declaring const constructors on @immutable classes.

[FatulM]

Hi, I've fixed analyzer version in version 1.3.0. And now all factories are constant factories for constant immutable sub classes. For your suggestions about maps and getter, I'll check them at first oppotunity. Thanks.

[FatulM]

For your first suggestion about maps: You mean we should use map instead of when ?

[FatulM]

And about getters, At first I had the same idea. But there are two points which stops me from using getters ... 1) using type casts is not a good Programming approach and we should not persuade it. By not using getters we somehow discourage user from using cast methods ! 😁 don't you agree ? 2) if we make them getters they may be mistaken with sub class fields, or possibly clash names.

[FatulM]

What's your opinion ? @iamarnas

[iamarnas]

For your first suggestion about maps: You mean we should use map instead of when ?

No, when and map is a different. map you need add when user generate all @WithWrap()

// When method. 
  R when<R extends Object?>({
    required R Function() loading,
    required R Function(Object error) error,
    required R Function(T data) data,
  }) {
    final result = this;
    if (result is ResultLoading<T>) {
      return loading();
    } else if (result is ResultError<T>) {
      return error(result.error);
    } else if (result is ResultData<T>) {
      return data(result.data);
    } else {
      throw AssertionError();
    }
  }

// Map method.
R map<R extends Object?>({
    required R Function() loading,
    required R Function(ResultError<T> error) error,
    required R Function(ResultData<T> data) data,
  }) {
    final result = this;
    if (result is ResultLoading<T>) {
      return loading();
    } else if (result is ResultError<T>) {
      return error(result);
    } else if (result is ResultData<T>) {
      return data(result);
    } else {
      throw AssertionError();
    }
  }

In short @WithWrap() is not maped data without is maped.

[iamarnas]

What's your opinion ? @iamarnas

@FatulM I agreed with you. Like I said, it's a taste thing and maybe and bad practice 😁

[FatulM]

About map, If I got your problem correctly, You have a situation which at the same time: 1) you need @WithWrap, (Which the only result of wrapping is changing when method and constructor signatures.) 2) you need when methods as you are not using @WithWrap

?

Or you need map and when simultaneously ? for example because some tool need map methods ...

A workaround is to generate map and mapOrElse methods when using withWrap as well as when not using withWrap ?

@iamarnas

[iamarnas]

@FatulM Example:

/// With this setup everything will be generated `@WithWrap()`. This should be as default.
@Sealed()
@WithWrap() /// You don't need at all this anotannion because `map` and `mapOrElse` in all case provide all data as map. 
abstract class _Result<T> {
  void loading();
  void data(T value);
  void error(Object error);
}

// All wrapped default methods as expected by setup.
 R when<R extends Object?>({
    required R Function() sunny,
    required R Function(T value) data,
    required R Function(Object error) error,
  }) {
    /* implementing whenOrElse and the rest... */

/// Add both `map`and `whenOrElse` methods only if you detect if user generate everything `@WithWrap()`
 R map<R extends Object?>({
    required R Function() sunny,
    required R Function(Result<T> value) data,
    required R Function(Result<T> error) error,
  }) {
    /*  Add whenOrElse */

This setup should be as default because map and mapOrElse in all case provide all data as map and you don't need annotation@WithWrap() at all or not generate maps methods if you detect that the user generates some function without a wrap, then when and map will be the same method. Example with Freezed you can do most with only with when , maybeWhen, map, maybeMap.

[iamarnas]

@FatulM

For example because some tool need map methods ...

map and mapOrElse in all case provide all data as map.

A workaround is to generate map and mapOrElse methods when using withWrap as well as when not using withWrap ?

Yes, or use may setup as default.

[iamarnas]

@FatulM in short. By adding map or mapOrElse you don't need @WithWrap() annotation at all because map and mapOrElse in all case provide all data as map. And you have everything in one place.

[FatulM]

Ok, I will consider this ... But it may take some time to implement it, And it will have breaking changes. Thanks a lot

[FatulM]

Hi, I changed implementation according to your recommendation. Please check out examples to see if it fits your needs. for example:

weather

@iamarnas

[iamarnas]

Hi, I changed implementation according to your recommendation. Please check out examples to see if it fits your needs. for example:

weather

@FatulM It looks amazing 👍. How to setup this? Or is this by default?

[FatulM]

Hi, I changed implementation according to your recommendation. Please check out examples to see if it fits your needs. for example: weather

@FatulM It looks amazing 👍. How to setup this? Or is this by default?

After I'll publish 1.5.0 it will be by default. I removed @WithWrap annotation. Now we have map methods like when not using WithWrap, And when methods like when using WithWrap.

If you were using WithWrap you should change all of factory and constructor calls to named args. Since I don't have any way to distinguish between wrapped and non wrapped state ...

[FatulM]

please check out changelog: changelog

I published 1.5.0

[iamarnas]

I will try as soon as I can 😊. As I understand you removed @WithWrap()? Just run builder and everything added in one place?

[iamarnas]

Hi @FatulM

Now I see a big difference and I am happy with this result. I fix the rest with small changes to fits my needs. Example:

Equality

Equality.distinct missing the hashCode, and I don't understand why you can't implement full code when you have all values.

Result by sealed generators:

/// Override `hashCode` if overriding `==`.
/// Note: also implemented in the classes without parameters.
@override
  bool operator ==(Object other) => false;

Should be:

///class [ResultError<T extends Object?, E extends Object?>]
@override
  bool operator ==(Object other) {
    if (identical(this, other)) return true;

    return other is ResultError<T, E> &&
      other.error == error &&
      other.stackTrace == stackTrace;
  }

  @override
  int get hashCode => error.hashCode ^ stackTrace.hashCode;

Default parameters

I don't know how Dart Build System builders work. But is that not possible to use the same parameters provided by the user. That should be enough only to remove default values from the factory constructor and from all methods. The rest just copy and paste.

@Sealed()
abstract class _Result<T, E> {
  void loading();
  // Default constructor by the user => (E error, [StackTrace stackTrace = StackTrace.empty])
  void error(E error, [StackTrace stackTrace = StackTrace.empty]); 
  void data(T value);
}

const factory Result.error(E error, [StackTrace stackTrace]) = ResultError<T, E>; // <= remove default value from factory constructors

class ResultError<T extends Object?, E extends Object?> extends Result<T, E> {
  // Paste default constructor (E error, [StackTrace stackTrace = StackTrace.empty])
  const ResultError(
    this.error, [
    this.stackTrace = StackTrace.empty,
  ]) : super._internal();

  final E error;
  final StackTrace stackTrace;

  @override
  String toString() => 'Result.error(error: $error, stackTrace: $stackTrace)';
}

R when<R extends Object?>({
    required R Function() loading,
    required R Function(E error, [StackTrace stackTrace]) error, // <= remove default value from the methotds
    required R Function(T value) data,
  }) {
  /*...*/

/// And in the functions need detect optional parameters type `[]` or `{}`
if (result is ResultError<T, E>) {
      return error(result.error, result.stackTrace); // or error(result.error, stackTrace: result.stackTrace);
} 
/*...*/

Otherwise is good work you can close this issue if you want. It's just my suggestion.

[FatulM]

about distinct equality: overriding hashcode does not make sense since all of instances are not equal to each other. even an instance is not equal to itself. what should I return from hashcode ? If i return a single result for example 0 or runtimeType.hashcode or superclassType.hashcode. (which is i think the last one is best). It does not make sense !

If you want that type of equals and hashcode like equatable package you should use data equality. This is like the functionallity which you have proposed ...

(There are three types of equality please check their implementation in example folder)

What is your opinion ?

[FatulM]

And please make another issue about default parameters and copy your recommendation there. I should investigate the difficulty of implementing that feature. I thin it is possible but is very hard to implement 😄

[FatulM]

And also I have changed cast methods to getters on master

[FatulM]

Thanks for your great suggestions

[iamarnas]

I'm not agreed with you about distinct equality. As you see it is very important for not primitive objects and more important for creating tests.

Just copy and paste in the dart pad to test it.

void main() {
  /// [Counter] with equality operator;
  print(ResultData(Counter(1)) == ResultData(Counter(1))); // true
  print(ResultData(Counter(1)) == ResultData(Counter(2))); // false
  print(ResultData(1) == ResultData(1)); // true
  print(ResultData(1) == ResultData(2)); // false

  // [Counter] without equality operator;
  print(ResultData(Counter(1)) == ResultData(Counter(1))); // false
  print(ResultData(Counter(1)) == ResultData(Counter(2))); // false
  print(ResultData(1) == ResultData(1)); // true
  print(ResultData(1) == ResultData(2)); // false

  /// And everything is `false` if [ResultData] does not have an equality operator.
}

class Counter {
  int value;
  Counter(this.value);

  @override
  String toString() => 'Result.data(value: $value)';

  @override
  bool operator ==(Object other) {
    if (identical(this, other)) return true;

    return other is Counter && other.value == value;
  }

  @override
  int get hashCode => value.hashCode;
}

// Generated by sealed generators
class ResultData<T extends Object?> extends Result<T> {
  const ResultData(this.value) : super._internal();

  final T value;

  @override
  String toString() => 'Result.data(value: $value)';

 // Added by me
  @override
  bool operator ==(Object other) {
    if (identical(this, other)) return true;

    return other is ResultData<T> &&
        other.value == value;
  }

  @override
  int get hashCode => value.hashCode;
}

[iamarnas]

I think, pure Dart equality operator would be as default. You really don't need depend you package on Equatable for that, keep your package clean as possible, it better for you and users. You can generate EquatableMixIn but allow to users if they want use it, they can install package self.

[FatulM]

Maybe it is a good approach, but in 6thsolution we mostly use this package with bloc, so we need data equality as default

If you want default behavior use Equality.identity

implementing data equality and hashcode by ourself is a goal of dart_sealed, but it will not be in near future

[iamarnas]

In 6thsolution we mostly use this package with Bloc.

Oh, I did not know that this package you created for yourself. Then I close this issue 😃 Thanks anyway for great a package.

[FatulM]

This is not a private package! And I will consider your recommendation ... but it may take some time. thanks.