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

Disallow adding non-factory constructors #6

Closed
chikamichi opened this issue May 31, 2023 · 4 comments · Fixed by #10
Closed

Disallow adding non-factory constructors #6

chikamichi opened this issue May 31, 2023 · 4 comments · Fixed by #10
Labels
enhancement New feature or request

Comments

@chikamichi
Copy link

Hi,

I have some code available on GitHub showcasing a weird issue.

Context: refactoring a working app to leverage DDD, with proper SoC between:

  • features (what the end-user does)
    • which leverage widgets (what the end-user sees and can interact with)
      • which leverage widgets controllers (indirection sublayer in the Presentation layer)
        • which (in this POC) are Riverpod async providers for modddels (in the Domain layer), with eventually remote data being fetched through a DataMapper layer

I got a union of modddels (source) named MapLayer, with three unioned "variants": MapLayer.raster, MapLayer.vector and MapLayer.geojson. I applied your insights from #4 and got it working… until I try to get a modddel instance.

On this specific call I get the following exception: UnsupportedError (Unsupported operation: It seems like you constructed your class using MyClass._(), or you tried to access an instance member from within the annotated class.)

My understanding is that it would only ever be raised only if trying to call MapLayer._() or an instance method such as map on a raw MapLayer instance, but it’s not what I’m doing here: I’m instantiating through the factory MapLayer.raster.

What am I missing?

Thank you!

@CodingSoot
Copy link
Owner

CodingSoot commented May 31, 2023

Hi,

The problem is in your Percentage modddel :

class Percentage extends SingleValueObject<InvalidPercentage, ValidPercentage>
    with _$Percentage {
  Percentage._();

  factory Percentage(double value) {
    return _$Percentage._create(
      value: value,
    );
  }
  
  /// ⚠️ The problem is here
  Percentage.maxed() {
    Percentage(1.0);
  }

  // ...
}

Here, you're declaring a named constructor Percentage.maxed. Calling it is the same as calling Percentage._() : it creates a blank unusable instance of Percentage, and that's why the error is thrown. In the constructor body, you properly create a Percentage instance, but you don't use it.

What you want to achieve is not possible, as normally you would need to use a named factory constructor, something like this :

factory Percentage.maxed() {
   return Percentage(1.0);
}

However, named factory constructors are reserved for creating unions of modddels. What I would suggest is to instead use a static method or getter :

static Percentage maxed() => Percentage(1.0);

// or
static Percentage get maxed => Percentage(1.0);

I think I should completely disallow adding non-factory constructors to a modddel class, as I can't see any valid usecase for them.

@CodingSoot
Copy link
Owner

@chikamichi Let me know if this fixed your issue

@CodingSoot CodingSoot changed the title Cannot instantiate through a factory: _.() gets called somehow Disallow adding non-factory constructors May 31, 2023
@CodingSoot CodingSoot added the enhancement New feature or request label May 31, 2023
@chikamichi
Copy link
Author

Thank you very much! You were right.

I was in the process of uncovering the exact stacktrace for I had a feeling MapLayer was not the actual culprit… you saved me some time again ;)

I think I should completely disallow adding non-factory constructors to a modddel class, as I can't see any valid usecase for them.

Agreed… until someone finds a legit use-case ;) Maybe just some warning/insight in the doc?

@CodingSoot
Copy link
Owner

Agreed… until someone finds a legit use-case ;) Maybe just some warning/insight in the doc?

I've decided to completely disallow non-factory constructors, as they serve no practical purpose and could potentially confuse newcomers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants