4

I am doing the below to get certain nodes from a treeview followed by getting text from those nodes, filtering text to remove unique and then appending custom image to the duplicate nodes.

For this I am having to loop 4 times. Is there is a simpler way of doing this? I am worried about it's performance for large amount of data.

//Append duplicate item nodes with custom icon
function addRemoveForDuplicateItems() {
    var treeView = $('#MyTree').data('t-TreeView li.t-item');
    var myNodes = $("span.my-node", treeView);
    var myNames = [];

    $(myNodes).each(function () {
        myNames.push($(this).text());
    });

    var duplicateItems = getDuplicateItems(myNames);

   $(myNodes).each(function () {
        if (duplicateItems.indexOf($(this).text()) > -1) {
            $(this).parent().append(("<span class='remove'></span>"));
        }
    });
}

//Get all duplicate items removing unique ones
//Input [1,2,3,3,2,2,4,5,6,7,7,7,7] output [2,3,3,2,2,7,7,7,7] 
function getDuplicateItems(myNames) {
    var duplicateItems = [], itemOccurance = {};

    for (var i = 0; i < myNames.length; i++) {
        var dept = myNames[i];
        itemOccurance[dept] = itemOccurance[dept] >= 1 ? itemOccurance[dept] + 1 : 1;
    }
    for (var item in itemOccurance) {
        if (itemOccurance[item] > 1)
            duplicateItems.push(item);
    }
    return duplicateItems;
}

4 Answers 4

0

If I understand correctly, the whole point here is simply to mark duplicates, right? You ought to be able to do this in two simpler passes:

var seen = {};
var SEEN_ONCE = 1;
var SEEN_DUPE = 2;

// First pass, build object
myNodes.each(function () {
    var name = $(this).text();
    var seen = seen[name];
    seen[name] = seen ? SEEN_DUPE : SEEN_ONCE;
});

// Second pass, append node
myNodes.each(function () {
    var name = $(this).text();
    if (seen[name] === SEEN_DUPE) {
        $(this).parent().append("<span class='remove'></span>");
    }
});

If you're actually concerned about performance, note that iterating over DOM elements is much more of a performance concern than iterating over an in-memory array. The $(myNodes).each(...) calls are likely significantly more expensive than iteration over a comparable array of the same length. You can gain some efficiencies from this, by running the second pass over an array and only accessing DOM nodes as necessary:

var names = [];
var seen = {};
var SEEN_ONCE = 1;
var SEEN_DUPE = 2;

// First pass, build object
myNodes.each(function () {
    var name = $(this).text();
    var seen = seen[name];
    names.push(name);
    seen[name] = seen ? SEEN_DUPE : SEEN_ONCE;
});

// Second pass, append node only for dupes
names.forEach(function(name, index) {
    if (seen[name] === SEEN_DUPE) {
        myNodes.eq(index).parent()
            .append("<span class='remove'></span>");
    }
});
0

The approach of this code is to go through the list, using the property name to indicate whether the value is in the array. After execution, itemOccurance will have a list of all the names, no duplicates.

var i, dept, itemOccurance = {};
for (i = 0; i < myNames.length; i++) {
    dept = myNames[i];
    if (typeof itemOccurance[dept] == undefined) {
        itemOccurance[dept] = true;
    }
}
0

If you must keep getDuplicateItems() as a separate, generic function, then the first loop (from myNodes to myNames) and last loop (iterate myNodes again to add the span) would be unavoidable. But I am curious. According to your code, duplicateItems can just be a set! This would help simplify the 2 loops inside getDuplicateItems(). @user2182349's answer just needs one modification: add a return, e.g. return Object.keys(itemOccurance).

0

If you're only concerned with ascertaining duplication and not particularly concerned about the exact number of occurrences then you could consider refactoring your getDuplicateItems() function like so:

function getDuplicateItems(myNames) {
    var duplicateItems = [], clonedArray = myNames.concat(), i, dept;
    for(i=0;i<clonedArray.length;i+=1){
        dept = clonedArray[i];
        if(clonedArray.indexOf(dept) !== clonedArray.lastIndexOf(dept)){              
           if(duplicateItems.indexOf(dept) === -1){
              duplicateItems.push(dept);
           }
           /* Remove duplicate found by lastIndexOf, since we've already established that it's a duplicate */
           clonedArray.splice(clonedArray.lastIndexOf(dept), 1);  
        }
    }
    return duplicateItems;
}

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.