refactor: Change the ClipComponent factory Constructor to redirect Constructor #3089
Conversation
YukiAttano
changed the title
YukiAttano
changed the title
YukiAttano
changed the title
YukiAttano
changed the title
YukiAttano
changed the title
YukiAttano
changed the title
| static ShapeBuilder _polygonBuilderCallback(List<Vector2> points) { | ||
| assert( | ||
| points.length >= 3, | ||
| 'PolygonClipComponent requires at least 3 points.', | ||
| ); | ||
|
|
||
| return (Vector2 size) => _polygonBuilder(points, size); | ||
| } | ||
|
|
||
| static Shape _polygonBuilder(List<Vector2> points, Vector2 size) { | ||
| final translatedPoints = points | ||
| .map( | ||
| (p) => p.clone()..multiply(size), | ||
| ) | ||
| .toList(); | ||
| return Polygon(translatedPoints); | ||
| } |
There was a problem hiding this comment.
I would merge these two methods and call the resulting method _polygonBuilder, since it isn't really a callback.
There was a problem hiding this comment.
The idea behind them is, that the first gets executed during initialization and therefor the assertion is run imediatly while it then returns the builder function that is used later.
If the assertion is in the builder itself, it gets first executed in the onLoad() method if i am right.
There was a problem hiding this comment.
Right, that makes sense, the second function could still be inlined, but now when looking at it that will be a bit more expensive so probably worth having it like it is now, but coming up with a better name for _polygonBuilderCallback. As always, hard with naming though. 😅
There was a problem hiding this comment.
Maybe just _polygonShapeBuilder?
There was a problem hiding this comment.
I'm in hurry but i think they can be merged as there is no way in changing the points for the polygon after calling the constructor. Therefor the assertion would lead back to the source of the error.
Anyway both implementations have pro/cons and are fine for now :)
There was a problem hiding this comment.
I'll fix it up then, since you're in a hurry :)
| static ShapeBuilder _polygonBuilderCallback(List<Vector2> points) { | ||
| assert( | ||
| points.length >= 3, | ||
| 'PolygonClipComponent requires at least 3 points.', | ||
| ); | ||
|
|
||
| return (Vector2 size) => _polygonBuilder(points, size); | ||
| } | ||
|
|
||
| static Shape _polygonBuilder(List<Vector2> points, Vector2 size) { | ||
| final translatedPoints = points | ||
| .map( | ||
| (p) => p.clone()..multiply(size), | ||
| ) | ||
| .toList(); | ||
| return Polygon(translatedPoints); | ||
| } |
There was a problem hiding this comment.
I'll fix it up then, since you're in a hurry :)
Description
The ClipComponent factory Constructor "circle", "rectangle" and "polygon" are now redirect Constructor while they persist their previous behaviour and syntax.
This allows subclasses of ClipComponent to directly address them.
Checklist
docsand added dartdoc comments with///.examplesordocs.Breaking Change?
Related Issues
Closes #3083