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

separate or rename dot.Util #4

Open
pixelzoom opened this issue May 31, 2013 · 9 comments
Open

separate or rename dot.Util #4

pixelzoom opened this issue May 31, 2013 · 9 comments

Comments

@pixelzoom
Copy link
Contributor

JO was considering moving each function in dot.Util to a separate file. Though having them all in one file is a little more convenient when you're going to use multiple functions, only requires one import.

If we don't separate them, then this should be renamed to something like MathUtils. var Util = require("DOT/Util") doesn't really tell me what Util is when I'm reading code.

@samreid
Copy link
Member

samreid commented Sep 20, 2013

I don't feel too strongly about it one way or the other. Maybe keep it in a single file (DotUtil or MathUtil) for now and re-evaluate if it starts getting too big?

@pixelzoom
Copy link
Contributor Author

This issue has been open for 5 months and dot.Util usage is proliferating. Let's make a decision one way or the other.

@jonathanolson
Copy link
Contributor

Due to the number of functions (and that it is growing), I would like these separated out into separate files.

I'm mostly unsure of what the desired packaging / files should be. Do we want DOT/polynomial/solveCubicRootsReal, etc.? Or just DOT/util/solveCubicRootsReal for everything that is in Util right now? Other naming strategies?

@samreid
Copy link
Member

samreid commented Oct 18, 2013

First of all, dot.Util isn't too big yet (currently 330 lines, many of which are comments). Second, what about splitting into a few different util files?

NumberUtil:
clamp
moduloBetweenDown
moduloBetweenUp
rangeInclusive
toRadians
toDegrees
cubeRoot

GeometryUtil:
lineLineIntersection
sphereRayIntersection
lineSegmentIntersection
distToSegmentSquared
distToSegment

FunctionUtil:
solveQuadraticRootsReal
solveCubicRootsReal

Not sure where to put linear, and I expect it to be commonly used, perhaps it could be its own file as suggested above?

Also, I'm not opposed to one file per function, but thought this might be a fair medium. Let's discuss.

@pixelzoom
Copy link
Contributor Author

Too bad we don't have something that strips out unused functions, that would make this a non-issue and maintain the convenience of importing one file.

I'm totally unsure on whether splitting things up a bit like @samreid described would be a good first step, or whether we should just proceed directly to a file-per-function.

@jonathanolson
Copy link
Contributor

I have a preference here for splitting utility functions generally into separate files (so they can be imported with their own name), and I'm willing to put in the work for it.

@pixelzoom
Copy link
Contributor Author

I'm pretty sure that this will never happen now, due to how painful the refactor would be.

@jonathanolson
Copy link
Contributor

I might try to do one function moved out a week, that probably wouldn't be too bad.

@samreid
Copy link
Member

samreid commented Apr 6, 2016

I can't see where in this issue thread it became high priority. Also, the last discussion is nearly a year old. I'm going to demote priority. If @jonathanolson or anyone else wants to self assign or change priority, go for it!

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

3 participants