-
Notifications
You must be signed in to change notification settings - Fork 6
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
Improve LinearFunction API #112
Comments
I'm planning to turn LinearFunction into a class with an |
Everything is passing aqua locally, I think this is ready for commit. |
I wasn't sure who to ask for review, so I ran
@jessegreenberg can you please review? Close if all is well. |
On second thought, I noticed the original author of LinearFunction was @pixelzoom, so perhaps @pixelzoom should review. The problem is described in #112 (comment) and the proposed solution (which was implemented) is described in #112 (comment). |
The proposed solution sounds great to me. I've always hated the original implementation - which was a consensus as I recall, to make call sites less verbose. |
Thanks, closing. |
Upon reflection, it is unclear whether @pixelzoom reviewed the implementation or the proposal only. @pixelzoom can you clarify? Please close if all is well. |
Implementation looks good, closing. |
During TypeScript porting with @jbphet, we saw that TypeScript was confused by our usage of LinearFunction. We say it is a
@constructor
and invoke it withnew
but we return a different function from the constructor. This causes type errors in TypeScript. We found things get better if we remove@constructor
andnew
. We may also want to return a rich object with "evaluate" and "inverse" methods, or rename it tocreateLinearFunction
.The text was updated successfully, but these errors were encountered: