ngraph.graph icon indicating copy to clipboard operation
ngraph.graph copied to clipboard

Change getLink/hasLink to also find link by id

Open fecorreiabr opened this issue 3 years ago • 1 comments

This pr is a proposal to improve getLink/hasLink so that it becomes possible to get a link by its id if only one argument is passed to these methods. It seems to work fine in my tests.

With this change it would be possible to get a link both by from-to NodeId and by LinkId:

var graph = createGraph();
var linkId = graph.addLink(1, 2).id; // no need to externally store the link, just its id
...
var link12 = graph.getLink(linkId); // link found by its id
var link12a = graph.getLink(1, 2); // 2 args are passed, keeps current behavior

Maybe this improvement also makes it possible to retrieve the links by id with multigraph option enabled, but I haven't tested this use case yet.

If this is accepted I think docs should be updated to reflect it.

closes #48

fecorreiabr avatar Oct 07 '22 13:10 fecorreiabr

sorry, just noticed this one. I think the need for the method makes sense, though the use of polymorphism with overloaded arguments looks a little bit risky to me: someone might pass second argument incorrectly and the behavior of the method drastically changes.

A bit more secure method that prevents bugs would be an explicit method, e.g. getLinkById(). What do you think?

anvaka avatar Aug 14 '23 21:08 anvaka