search

kchapelier / node-mathp

Math utility for node
MIT License
9 stars 1 forks source link

Don't bind everything #4

Closed megawac closed 9 years ago

megawac commented 9 years ago

It would be much more performant to not bind all the functions but keep an internal reference to the ones that require a binding. Otherwise you can do a test to see if binding is necessary (assign to me if you want me to set that up)

Example of perf lose

http://jsperf.com/mathp-binding

kchapelier commented 9 years ago

Actually, I've been thinking about changing the whole structure of the code base for a while.

Currently, the functions are lumped together in litteral objects which are in their own file and then merged in the mathp object. I'm planning to have each function in a different file which would then require any functions it depends upon (instead of expecting it to be available on this). The functions would then simply be added to the Mathp object without any binding.

It would also have the added benefit of allowing "deep require"

var dist = require('mathp/lib/euclideanDistance');
console.log(dist(0, 0, 12, 15));
megawac commented 9 years ago

Yeah, that would be better, and you can "require" the Math functions you need to delegate to at the same time

var min = Math.min; :)

On Sat, Dec 13, 2014 at 3:03 PM, Kevin Chapelier notifications@github.com wrote:

Actually, I've been thinking about changing the whole structure of the code base for a while.

Currently, the functions are lumped together in litteral objects which are in their own file and then merged in the mathp object. I'm planning to have each function in a different file which would then require any functions it depends upon (instead of expecting it to be available on this). The functions would then simply be added to the Mathp object without any binding.

It would also have the added benefit of allowing "deep require"

var dist = require('mathp/lib/euclideanDistance');console.log(dist(0, 0, 12, 15));

— Reply to this email directly or view it on GitHub https://github.com/kchapelier/node-mathp/issues/4#issuecomment-66889172.

kchapelier commented 9 years ago

Started the work on the restructure branch

megawac commented 9 years ago

Its looking good!

kchapelier commented 9 years ago

The build system had to be rethought about (for the better) but the branches are now merged. It will be pushed to npm in a few.

I updated the jsperf to compare the old and new implementations ( http://jsperf.com/mathp-binding/3 ). It's about 99% (!) better for call to generic Math functions. Thanks for raising the issue.