d3-sankey-circular icon indicating copy to clipboard operation
d3-sankey-circular copied to clipboard

modular source code?

Open micahstubbs opened this issue 7 years ago • 6 comments

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 :smile:)

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

micahstubbs avatar May 29 '18 18:05 micahstubbs

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.

tomshanley avatar May 29 '18 20:05 tomshanley

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

micahstubbs avatar May 29 '18 21:05 micahstubbs

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.

micahstubbs avatar May 29 '18 22:05 micahstubbs

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

tomshanley avatar May 29 '18 22:05 tomshanley

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

peteruithoven avatar Aug 29 '19 13:08 peteruithoven

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

micahstubbs avatar Sep 01 '19 22:09 micahstubbs