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

modular source code? #17

Open
micahstubbs opened this issue May 29, 2018 · 6 comments
Open

modular source code? #17

micahstubbs opened this issue May 29, 2018 · 6 comments

Comments

@micahstubbs
Copy link
Contributor

I want to break sankeyCircular.js up into may small files so that the interfaces between the functions it contains are more explicit to me.

this is an activity mainly aimed at getting myself familiar with d3-sankey-circular. when I'm happy with it, I'll send a PR to see if this code organization approach is useful to others. (if not, I'm perfectly happy doing this purely as a learning exercise 😄)

I make an issue because old habits compel me to include an issue number in my branch names 😂

@tomshanley
Copy link
Owner

Hi Micah - thank you, it would definitely benefit from someone who knows more about modular code, and using bundle etc than me :)

An aside, i'm planning an update (still early stages) that would allow a drawn sankey to be updated with new values, without affecting the overall layout too much (ie the nodes and links would remain pretty much in their position, but with new heights/widths). so it would skip a lot of the initial positioning of nodes/links, update the heights and access the functions to generate the new circular path d. so making some parts more modular would be great help towards that.

@micahstubbs
Copy link
Contributor Author

ah interesting. good hear that this generally fits with what you have in mind 😄

@micahstubbs
Copy link
Contributor Author

micahstubbs commented May 29, 2018

my rough criteria for abstracting a function out into it's own file seem to be:

if the function is:

1) more than 5 lines of code long
2) is called by multiple other functions

this seems to be optimal for me to get a sense of the dependencies inside the library and easily read the code. this may or may not be optimal for other things that are important to this library.

@tomshanley
Copy link
Owner

sounds reasonable.. for the update function I mentioned earlier, I'm probably going to need to use addCircularPathData and its dependents (calcVerticalBuffer and createCircularPathString, while updating addCircularPathData to avoid any sorting functions), and probably adapt this adjustNodeHeight

@peteruithoven
Copy link
Contributor

@micahstubbs any updates on this?
It might be interesting to check how d3-sankey-diagram is broken up.

@micahstubbs
Copy link
Contributor Author

@peteruithoven no update. thanks for the signal that it could be useful for you - I may take another look at this. cheers!

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

No branches or pull requests

3 participants