atp-mipt/ljv

Adding multiple objects to same graph Instance

Closed this issue · 14 comments

How do I show the relation between multiple Objects?

Let's say I have a class :

public class Node {

    int data = 5;

}

The only way to show how n1,n2,n are connected is shown below

public static void main(String[] args) {
       Node n = new Node();

       Node n1 = n;
       Node n2 = n;


       System.out.println(new LJV().drawGraph(new Object[]{n1,n2,n}));
   }

It would be great If I can do it as below.

LJV ljv = new LJV();

ljv.add(n);
ljv.add(n1);
ljv.add(n2);

ljv.drawGraph();

is this possible currently?

please let me know.

Hi @bhanuunrivalled , thanks for the issue!

Yes I thought about this feature, but I didn't came to an elegant solution.

ljv.add(n);
ljv.add(n1);
ljv.add(n2);

ljv.drawGraph();

looks too verbose,

I thought about having ljv.drawGraph(Object... objects), but this makes it impossible to distinguish between many objects and an actual array (because if I pass an array, it will be treated as varargs). Maybe we should have different methods, like

  • drawTree(Object o) -- the graph will always have a single root, this can be used if we want to visualize an actual array
  • drawGraph(Object ... o) -- if we pass only one argument which is not an array, it will be equivalent to drawTree, if we pass multiple arguments OR a single array argument -- this gives a graph with many root objects

This should be easy to implement.

What do you think?

Hi @bhanuunrivalled , I just came up with another idea: since LJV already uses setters that return this, we can avoid messing with varargs and utilize method chaining API:

new LJV().addRoot(n1).addRoot(n2).drawGraph();

or

new LJV().addRoot(n1).drawGraph(n2);

What we can do:

  • add "chaining operator" LJV addRoot(Object o) that adds the new object to the identity set of root objects
  • add overloaded "terminal operator" String drawGraph() that visualizes the graph from the provided root nodes
  • preserve the old version of String drawGraph(Object o) for the sake of simplicity and backwards compatibility, but it will be equivalent of calling addRoot(o).drawGraph().

Your thoughts?

HI @inponomarev,

new LJV().addRoot(n1).drawGraph(n2);

I like this idea, considering the backward compatibility.

Yeah, I think this API design is good

This must be easy to implement, so if you really like to contribute you may implement it yourself

(we'll just have to maintain internal List<Object> of root objects for LJV class, and GraphBuilder.generateDot should become parameterless and just iterate over all the elements of this list)

Sure I will do it Thank you.

How about:

drawGraph(Object o);
drawAll(Object... roots);

I like the proposed naming :-) Maybe we can implement just everything -- drawGraph(Object o), drawAll(Object... roots) and addRoot(Object o) !!

I still like the idea of addRoot since it simplifies the dynamic addition of arbitrary number of roots in a for loop. Otherwise you'll have to collect the roots to a list and then convert the list to an array to pass it as varargs.

HI @inponomarev @orionll,

please review the pull request and let me know your ideas.

you can start from the test case org.atpfivt.ljv.LJVTest#multipleRoots.

When I write new LJV().addRoot(1).addRoot(2).drawGraph(), the graph looks like this:

addRoot_1_2

What's the number 1 here in the graph?

I think the graph should look like this:

addRoot_1_2_correct

Also, when I call drawGraph(Object), the graph should include all roots rather than only the root specified in drawGraph(Object).

I would like to ask, do we really need so many flavors of draw* methods? JOL has happily lived with a single GraphLayout.parseInstance(Object... roots). I think we should do the same and have a single method:

public String drawGraph(Object... roots);

If someone wants to draw an array, they can do a cast: drawGraph((Object) array).

addRoot is still useful, so I think we should keep it too.

I am counting the number of references . Can you pls once check the test case I wrote

When I write new LJV().addRoot(1).addRoot(2).drawGraph(), the graph looks like this:

addRoot_1_2

What's the number 1 here in the graph?

I think the graph should look like this:

addRoot_1_2_correct

Also, when I call drawGraph(Object), the graph should include all roots rather than only the root specified in drawGraph(Object).

HI @orionll , The issue I reported is different , my concern is how does the graph handle a group of objects in which some of them are referring to the same memory location . please once refer my Test case
@inponomarev please check once my test case

Hi everyone! @bhanuunrivalled I have merged your code -- but I had to do some edits, that's why I opened a separate PR, that's why your original PR was closed, but your code and your authorship is there! Thank you very much for your input for LJV.

I also released version 1.03, so starting from 1.03 there will be an option to add many roots to the graph.

There was an argument in this thread about API and how should we visualize the multiple roots. I'll use my authority as a primary maintainer of the project and decide the following:

  • I fully agree with @orionll point that there is no reason in 'indexing' the roots of the graph. I removed the 'indexing' nodes, so the graph with multiple roots will look simply like

изображение

  • If one needs to index roots, one can use an array, like in this example:
        String[] s = new String[]{x,  new String(x),
                new String(x.toCharArray()), x + "", "" + x, x.concat(""), "".concat(x)};

изображение

  • Concerning @orionll proposal. Due to historical reasons, it's very common to use LJV to visualize arrays (like in the example above). Examples based on array visualization have been provided by John Hamer since 2004, in my recent presentation there are plenty of array-based examples. That's why it's important to keep array visualization simple and it's better to avoid varargs for multiple roots. After all, avoiding varargs will keep backwards compatibility!

I'm closing this issue now, versions since 1.03 will have multi-rooted graphs.

Hi everyone! @bhanuunrivalled I have merged your code -- but I had to do some edits, that's why I opened a separate PR, that's why your original PR was closed, but your code and your authorship is there! Thank you very much for your input for LJV.

I also released version 1.03, so starting from 1.03 there will be an option to add many roots to the graph.

There was an argument in this thread about API and how should we visualize the multiple roots. I'll use my authority as a primary maintainer of the project and decide the following:

  • I fully agree with @orionll point that there is no reason in 'indexing' the roots of the graph. I removed the 'indexing' nodes, so the graph with multiple roots will look simply like

изображение

  • If one needs to index roots, one can use an array, like in this example:
        String[] s = new String[]{x,  new String(x),
                new String(x.toCharArray()), x + "", "" + x, x.concat(""), "".concat(x)};

изображение

  • Concerning @orionll proposal. Due to historical reasons, it's very common to use LJV to visualize arrays (like in the example above). Examples based on array visualization have been provided by John Hamer since 2004, in my recent presentation there are plenty of array-based examples. That's why it's important to keep array visualization simple and it's better to avoid varargs for multiple roots. After all, avoiding varargs will keep backwards compatibility!

I'm closing this issue now, versions since 1.03 will have multi-rooted graphs.

Thank you. Good to know some of the code is helpful. I will check the new version.