31

I recently interviewed for a frontend engineer position. For my phone screen I was the following question: Given a node from a DOM tree find the node in the same position from an identical DOM tree. See diagram below for clarity.

 A         B

 O        O
 |\       |\
 O O      O O
  /|\      /|\
 O O O    O O O
      \        \
       O        O

Here was my solution, I was wondering what I could have done to improve/optimize it.

var rootA, rootB;

function findNodeB(nodeA) {
    // Variable to store path up the DOM tree
    var travelPath = [];
    
    // Method to travel up the DOM tree and store path to exact node
    var establishPath = function(travelNode) {
        // If we have reached the top level node we want to return
        // otherwise we travel up another level on the tree
        if (travelNode === rootA) {
            return;
        } else {
            establishPath(travelNode.parentNode);
        }
        
        // We store the index of current child in our path
        var index = travelNode.parentNode.childNodes.indexOf(travelNode);
        travelPath.push(index);     
    }
    
    var traverseTree = function(bTreeNode, path) {
        if(path.length === 0) {
            return bTreeNode;
        } else {
            traverseTree(bTreeNode.childNodes[path.pop()], path);
        }
    }
    
    establishPath(rootB, nodeA);
    
    return traverseTree(rootB, travelPath);
}           
       
       
thedjpetersen
  • 27,857
  • 4
  • 26
  • 28
  • 1
    You didn't get the job? – alex Nov 04 '13 at 23:44
  • I did not - the verbal feedback during the interview was good, so I figured I must of missed something with my solution. – thedjpetersen Nov 04 '13 at 23:45
  • Were you asked specifically to use recursion? Iterative would be much simpler in this case. Also, we have _zero_ information about the DOM trees/structures/element types? There is no information to work with except index position within the childNode array? – Mike Edwards Nov 04 '13 at 23:56
  • 1
    @Mike - I'd be interested to see an iterative approach to this, just to compare the differences in efficiency (and for learning purposes). – Axel Nov 05 '13 at 00:03
  • I like this question, but I think it might fit better on Programmers Stack Exchange? – Jordan Gray Nov 05 '13 at 00:04
  • 3
    Well, seeing that a `.childNodes` collection doesn't have an `indexOf()` method, I would guess that would count against you pretty strongly. – Blue Skies Nov 05 '13 at 00:09
  • 2
    Interesting note, that got me too. However, "count against you pretty strongly" only if the interviewer is a Nazi, in a real scenario you'd find that out pretty quick and work around with Array.prototype.indexOf.call() – Mike Edwards Nov 05 '13 at 00:14
  • Good point! I didn't know that - I am surprised that it wasn't pointed out, as at the end he said the solution looked good. – thedjpetersen Nov 05 '13 at 00:15
  • Maybe he counted it as pseudo code, since the intent is clear. – Blue Skies Nov 05 '13 at 00:17
  • The DOM is a graph structure so recursion comes here natively (though I don't understand why you'd use a closure here?). Also, why aren't you accepting `rootB` as a parameter? I think your solution is perfectly acceptable and there has been another factor for not hiring you. (maybe they already found someone?) – Benjamin Gruenbaum Nov 05 '13 at 01:27
  • Recursion makes more sense when you are _descending_ a tree and you need to follow multiple branches and maintain context while doing so. When you are _ascending_ the tree you don't need to leverage the callstack to record context for you and an iterative solution can be more self-contained. – Mike Edwards Nov 05 '13 at 01:30
  • Wait, nevermind - it doesn't work - I tried running it and got an exception. – Benjamin Gruenbaum Nov 05 '13 at 01:32
  • You have to realize the simple practical meaning of the question first. Practically, for example, the solution may able to alert the value of another select list option on selecting from a menu where the two menus in the same div. – SaidbakR Mar 02 '15 at 18:22
  • 1
    also, just to be nitpicky - establishPath's signature takes 1 argument, but you pass it 2, and traverseTree needs to return itself when it makes the recursive call (right now, it just makes a recursive call, but doesn't return anything) – dchang Sep 21 '15 at 00:52

4 Answers4

23

Since at least Axel showed interest in an iterative solution, here it is:

Given two trees which have identical structure, and a specified node within the first tree, locate the node in the second tree with the same position within the structure.

If we have no other information about the two trees then the position of each node can be characterized as a path from the root node where each step in the path is specified as an index into the childNode array.

function indexOf(arrLike, target) {
    return Array.prototype.indexOf.call(arrLike, target);
}

// Given a node and a tree, extract the nodes path 
function getPath(root, target) {
    var current = target;
    var path = [];
    while(current !== root) {
        path.unshift(indexOf(current.parentNode.childNodes, current));
        current = current.parentNode;
    }
    return path;
}

// Given a tree and a path, let's locate a node
function locateNodeFromPath(root, path) {
    var current = root;
    for(var i = 0, len = path.length; i < len; i++) {
        current = current.childNodes[path[i]];
    }
    return current;
}

function getDoppleganger(rootA, rootB, target) {
    return locateNodeFromPath(rootB, getPath(rootA, target));
}

EDIT: As Blue Skies observed, childNodes doesn't have .indexOf(). Updating with Array.prototype.indexOf.call()

Mike Edwards
  • 3,742
  • 17
  • 23
  • 1
    Nice, while we're code reviewing - there is no point in caching path.length since it's an array and not a NodeList so accessing .length is O(1) – Benjamin Gruenbaum Nov 05 '13 at 01:25
  • 2
    Two years later, shouldn't this be using `current.parentNode.children` since `.children` will return only HTML elements? It would appear that a call to `childNodes` contains [more than just HTML elements](http://stackoverflow.com/questions/16762027/difference-between-javascript-childnodes-children) – Morklympious Oct 21 '15 at 18:39
  • 2
    @Morklympious Not really cause if you used children then that wouldn't be the same structure, remember comments are part of the structure of the document. –  Jul 12 '16 at 09:02
7

Here is parallel traversing solution

function findDomNodeInTree(rootA, rootB, targetNode) {
    if (rootA === targetNode){
        return rootB;
    }

    let nodeB = null;

    for (let i=0; i<rootA.childNodes.length && nodeB === null; i++){
        nodeB = findDomNodeInTree(rootA.childNodes[i], rootB.childNodes[i], targetNode);
    }

    return nodeB;
}

It's time complexity is O(N) and in worst case we need to traverse entire tree.

I don't think it less efficient than solution with finding path first. At each level there is a call to indexOf which may need to traverse all nodes on that level to find the index.

Yuriy Kvartsyanyy
  • 2,656
  • 2
  • 14
  • 16
3

Instead of Array.prototype.indexOf.call, you can use Array.from (standardized in ES6):

Array.from(travelNode.parentNode.childNodes).indexOf(travelNode);

amilajack
  • 127
  • 1
  • 10
  • Good idea. The only trade off would be that prototype.index.call operates directly on the object while .from().indexOf() would create a shallow copy which would add to the space complexity. A trade off that would be easily explained in the interview. – Robby_rob Jul 01 '18 at 16:51
0

I would traverse the two trees in parallel and when I got to the node in treeA return the parallel node.

kennytilton
  • 994
  • 1
  • 6
  • 16
  • 3
    Don't you think that is inefficient? When you traverse **to the node**, you will come across branches that you will have to unnecessarily check, making the algorithm inefficient (imagine your node being in branch B, and branch A having 1000s of children/grandchildren). Rather if you traverse from the node to the root, this will be efficient. – Om Shankar Aug 05 '15 at 21:20