eles.add() array parameter

[Jacksoor]

There is a problem when using eles.add() with an array as a parameter: If the array contains a collection instead of just plain elements, cytoscape treats the collection as an element, tries to read data.id and fails to create a new return collection. A simple fix could be:

if(is.array(toAdd)){
    for(var i = 0; i < toAdd.length; i++){
        if(is.element(toAdd[i])){
            var add = !this._private.ids[ toAdd[i].id() ];
            if( add ){
                elements.push( toAdd[i] );
            }
        }
        else if(is.collection(toAdd[i])){
            for(var j = 0; j < toAdd[i].length; j++){
                var add = !this._private.ids[ toAdd[i][j].id() ];
                if( add ){
                    elements.push( toAdd[i][j] );
                }
            }
        }
    }
}
else{
    for( var i = 0; i < toAdd.length; i++ ){

    var add = !this._private.ids[ toAdd[i].id() ];
    if( add ){
        elements.push( toAdd[i] );
    }
}

replacing the existing for-loop in elesfn.add. If this behavior is intended, maybe add a note to the documentation that there shouldn't be any collections in the array parameter.

[maxkfranz]

The docs for cy.add() already has an explicit list of the inputs allowed: http://js.cytoscape.org/#cy.add

For the array case, it already says plain objects and links to the JSON format. I'm not sure how this could be made clearer.

I suppose cy.add() and cy.remove() could be more fault tolerant, but adding array of element support doesn't really have a compelling usecase as far as I can see. Most functions in the API return collections, and cy.collection() can make a collection from an array if need be.

Is there a reason why you're using arrays rather than collections? What's your usecase?

[Jacksoor]

The function/documentation I was referring to is http://js.cytoscape.org/#eles.union which has an alias eles.add().

My usecase is that I split up my graph in subsets using algorithms to traverse it in specific ways. To show the user which elements are currently active, I tried to create one collection containing all these subsets and since I doing something like subset.add(subset1.add(subset2.add( ...))) seemed odd to me, I tried using subset.add([subset1, subset2, ...]). The issue is that these subsets are generally collections and the only way to combine them again is to chain the add function.

The main problem in my case is that there's no function that combines collections/elements like collection.add(collection1, collection2, ...) which returns a new collection containing all elements.

However if this hasn't been an issue until now, I guess I just need to work my way around it.

[maxkfranz]

Usually, that's done something like this:

var all = cy.collection();

[ subset1, subset2, subset3 ].forEach(function( subset ){
  all = all.union( subset );
});

Or more concisely:

var all = [ sub1, sub2, sub3 ].reduce( function( coln, sub ){
  return coln.union(sub);
}, cy.collection() );

[pmall]

but adding array of element support doesn't really have a compelling usecase as far as I can see.

For example in my viewer I have buttons to expand network based on selected nodes. So I send an ajax request to retrieve the selected nodes neigbours, I use cy.add with the returned array of elements.

[maxkfranz]

Adding an array of element JSON is already supported: http://js.cytoscape.org/#cy.add

[Jacksoor]

Usually, that's done something like this

I know how it's done now, I just thought that it would be more comfortable for the developer and better performance wise, if you could just add multiple Elements/Collections to a Collection with one function call instead of calling union for each Element/Collection separately.

Anyway I just wanted to add this as an enhancement and you can close this issue if you don't consider this as something useful. Thanks for the replies.

[maxkfranz]

It's best to use the collections instead of arrays. If you use collections, then it's always one call: col1.union( col2 )

If you really want to use arrays, then you can just col1.union( cy.collection( array ) )

It might be more convenient if there's automatic sanitisation, but I think that would touch a lot of the API. I'll take a look, but I can't guarantee it would make it into the next feature release.

[Jacksoor]

I don't think you got my point quite right yet, so I'll make an example:

var col1 = cy.elements('[x=1]');
var col2 = cy.elements('[x=2]');
var col3 = cy.elements('[x=3]');
//I use a function for each of these collections returning a new collection
col1 = myFunction(col1);
col2 = myFunction(col2);
col3 = myFunction(col3);
//Now I want to combine all these collections into one collection
var total;
total = col1.add(col2,col3); //Opt.1: Doesn't work as intended
total = col1.add([col2,col3]); //Opt.2: Exception
total = col1.add(cy.collection(col2,col3)); //Doesn't work as intended
total = col1.add(cy.collection([col2,col3])); //Exception
total = col1.add(col2.add(col3)); //This works but gets quite complicated
...

Now imagine that I got a lot of collections in my code or an array of collections and I just want an easy and performant way to add them all at once, like with option 1 or 2.

[maxkfranz]

That doesn't really make sense in the API consistently. Set theory functions should be binary operations, each with an arity of two.

I thought you were using arrays of elements rather than arrays of large sized collections. In that case, you may as well just call .union() twice or use reduce() for longer cases. If you use a more functional style, then each reduce() call is very concise:

var union = function( a, b ){ return a.union(b); };
var empty = function(){ return cy.collection(); };
var multiunion = function(){ return Array.prototype.slice.call( arguments ).reduce( union, empty() ); };

var all = [ sub1, sub2, sub3 ].reduce( union, empty() ); 

// or...
all = multiunion( sub1, sub2, sub3 );