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

feat: update API to allow optional parameters #33

Open
alestiago opened this issue Feb 28, 2022 · 2 comments
Open

feat: update API to allow optional parameters #33

alestiago opened this issue Feb 28, 2022 · 2 comments

Comments

@alestiago
Copy link
Collaborator

alestiago commented Feb 28, 2022

Description

From flame, one can instantiate a component as follows:

 textComponent = TextComponent(
    text: counter.toString(),
    textRenderer: _textPaint,
  );

See implementation here.

For consistency, perhaps it would be nice to update forge2d API to also support this pattern. For example, with shapes. We could:

final shape = CircleShape(radius: radius);

instead of

final shape = CircleShape()
  ..radius = radius;

Additional Context
Personally, both can be seen as equivalent. I think both APIs should be supported (cascading and optional parameters). Besides consistency with flame, one could also argue that using the positional parameter avoids setting/changing the value first unnecessary, since we already know its computed value and will change instantly after.

In addition, we also benefit from the analyser when we are setting default values.

This issue is open to discussion. If we decide to proceed I'm willing to contribute with this update.

@spydon
Copy link
Member

spydon commented Feb 28, 2022

The problem is that we are losing consistency with Box2D if we do this. On the other hand maybe we can allow both?

Edit: properly read what you wrote now, sounds good. 🙂

@alestiago
Copy link
Collaborator Author

alestiago commented Feb 28, 2022

@spydon thanks for joining in!

Definitely want both! Should we go forward with this change?

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

No branches or pull requests

2 participants