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

Using Array.prototype.push() instead of Array.prototype.concat() in appropriate places #10

Open
metincansiper opened this issue May 29, 2022 · 0 comments
Assignees

Comments

@metincansiper
Copy link
Collaborator

metincansiper commented May 29, 2022

Array.prototype.concat() returns a new array so using it to add all element of an array to the other one would be inefficient in some cases. I listed the code segments where I believe it could worth updating that code segment to add each element using Array.prototype.push() instead of using Array.prototype.concat() (some of them are from this repository and some of them are from the layout-base repository):

I believe that the line below would not make a big difference but I think I can still add it to this list while mentioning the similar updates. I guess that it would perform slightly better if similar updates would be done there (I think it may be dependent on how large would source.getEdgeListToNode(target) be). Also, I recognized that LNode.prototype.getEdgeListToNode() already builds and returns a new list (https://github.com/iVis-at-Bilkent/layout-base/blob/9877008db719062ed89c1627d8c883386d9f0a4b/src/LNode.js#L144). Therefore, if I am not missing something, it would be safe to directly initialize the edgeList variable as var edgeList = source.getEdgeListToNode(target) above this line. I do not know if it would make a nontrivial difference but I can still mention that a similar case (just about initializing edgeList as empty vs assigning it directly to source.getEdgeListToNode(target)) would exist in the related java code as well (https://github.com/iVis-at-Bilkent/chilay/blob/605e77732a6f06da2081e2d89a68084c1e508cdb/src/main/java/org/ivis/layout/cose/CoSELayout.java#L388)

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

2 participants