Is this a decent sample implementation of javascript reflection?

Question

This is for a job interview and I'd love it if someone could put an eyeball on this and say if I'm making any big mistakes. Here was the question:

1) Assume we're developing a JS-based debugger, similar to Firebug, and we need a way to inspect JS objects and elements. Write a routing that takes an object/element as input and list out its properties (such as attributes, methods, etc.), values and data types.

Here's my implementation:

var printObjectReflection = function(showPrototypeChainProperties){

    // Default: false
    showPrototypeChainProperties = 
     typeof(showPrototypeChainProperties) != 'undefined' ? showPrototypeChainProperties : false;

    var objToReflect = this;

    // Create and populate collections for different lists
    var methodPropertyList = Array();
    var arrayPropertyList = Array();
    var objectPropertyList = Array();
    var scalarPropertyList = Array(); 

    for (property in objToReflect) {

     var propertyName = property;
     var propertyValue = objToReflect[property];
     var propertyType = typeof(objToReflect[property]);
     // For telling integer indexed arrays from other types of objects
     var isArray = objToReflect[property] instanceof Array;

     // Makes sure this is an actual property of the object and not something
     // from the prototype chain, unless show is specified
     if (objToReflect.hasOwnProperty(property) || showPrototypeChainProperties){

      //
      // Routing to populate lists:
      //

      // Methods
      if (propertyType === 'function') {
       methodPropertyList.push({"propertyName" : propertyName})

      // Arrays and other objects 
      } else if (propertyType === 'object') {   
       if (isArray) {
        arrayPropertyList.push({"propertyName" : propertyName})  
       } else {
        objectPropertyList.push({"propertyName" : propertyName})
       }

      // Scalar member variables
      } else {

       scalarPropertyList.push({
        "propertyName" : propertyName,
        "propertyValue" : propertyValue,
        "propertyType" : propertyType
       })

      }
     }

    }

    //
    // Cheap and dirty display
    // In real life, this would populate some kind of console or log
    // instead of simple document.write's
    //

    document.write('<h3>Methods in this object:</h3>');
    if (methodPropertyList.length > 0) {
     var i;
     for (i = 0; i < methodPropertyList.length; i += 1) {
      document.writeln("<strong>Method name:</strong> " +
           methodPropertyList[i].propertyName + "<br />");
     } 
    } else {
     document.writeln("No methods. <br /><br />"); 
    }

    document.write('<h3>Arrays in this object:</h3>');
    if (arrayPropertyList.length > 0) {
     var i;
     for (i = 0; i < arrayPropertyList.length; i += 1) {
      document.writeln("<strong>Array name:</strong> " +
           arrayPropertyList[i].propertyName + 
           "<br />");
     } 
    } else {
     document.writeln("No arrays. <br /><br />"); 
    }

    document.write('<h3>Non-array objects in this object:</h3>');
    if (objectPropertyList.length > 0) {
     var i;
     for (i = 0; i < objectPropertyList.length; i += 1) {
      document.writeln("<strong>Object name:</strong> " +
           objectPropertyList[i].propertyName + 
           "<br />");
     } 
    } else {
     document.writeln("No objects. <br /><br />"); 
    }

    document.write('<h3>Scalars for this object:</h3>');
    if (scalarPropertyList.length > 0) {
     var i;
     for (i = 0; i < scalarPropertyList.length; i += 1) {
      document.writeln("<strong>Name:</strong> " +
           scalarPropertyList[i].propertyName + 
           " | <strong>Value:</strong> " +
           scalarPropertyList[i].propertyValue +
           " | <strong>Data type:</strong> " +
           scalarPropertyList[i].propertyType +
           "<br />");
     } 
    } else {
     document.writeln("No scalar variables. <br /><br />"); 
    }

} // end function

And a sample use:

<h2>DOM Element to look for:</h2>
<input id="inputElementToLookFor" type="text" value="monkeyShines"  />

<h2>A created object literal:</h2>
<script type="application/javascript">
// An object we created
var testObj = {
    'first-name' : "dan",
    'aNumber' : 234,
    'anArray' : [1,2,3,4],
    'anObject' : {"one":1, "two":2},
    'aMethod' : function() {}
};

testObj.printObjectReflection = printObjectReflection;
testObj.printObjectReflection();
</script>

<h2>An HTML element we pulled from the DOM: </h2>
<script>
// An HTML element
var myInputElement = document.getElementById("inputElementToLookFor")
myInputElement.printObjectReflection = printObjectReflection;
myInputElement.printObjectReflection(true);
</script>

Any incredibly foolish errors anyone can spot? Thanks in advance for the skilled eyeball.

EDIT: Ok, here's my revised implementation. I did end up just completely removing the hasOwnProperty check entirely since it seems to choke IE terribly and it's not really a part of the requirements to limit based on hasOwnProperty:

var printObjectReflection = function(objToReflect){ 

    // Create and populate collections for different lists
    var methodPropertyList = [];
    var arrayPropertyList = [];
    var objectPropertyList = [];
    var scalarPropertyList = []; 

    for (property in objToReflect) {

     var propertyName = property;
     var propertyValue = objToReflect[property];
     var propertyType = typeof(objToReflect[property]);
     // For telling integer indexed arrays from other types of objects
     var isArray = objToReflect[property] instanceof Array;

     //
     // Routing to populate lists:
     //

     // Methods
     if (propertyType === 'function') {
      methodPropertyList.push({"propertyName" : propertyName})

     // Arrays and other objects 
     } else if (propertyType === 'object') {   
      if (isArray) {
       arrayPropertyList.push({"propertyName" : propertyName})  
      } else {
       objectPropertyList.push({"propertyName" : propertyName})
      }

     // Scalar member variables
     } else {

      scalarPropertyList.push({
       "propertyName" : propertyName,
       "propertyValue" : propertyValue,
       "propertyType" : propertyType
      })

     }
    }

    //
    // Cheap and dirty display
    // In real life, this would populate some kind of console or log
    // instead of simple document.write's
    //

    document.write('<h3>Methods in this object:</h3>');
    if (methodPropertyList.length > 0) {
     var i;
     for (i = 0; i < methodPropertyList.length; i += 1) {
      document.writeln("<strong>Method name:</strong> " +
           methodPropertyList[i].propertyName + "<br />");
     } 
    } else {
     document.writeln("No methods. <br /><br />"); 
    }

    document.write('<h3>Arrays in this object:</h3>');
    if (arrayPropertyList.length > 0) {
     var i;
     for (i = 0; i < arrayPropertyList.length; i += 1) {
      document.writeln("<strong>Array name:</strong> " +
           arrayPropertyList[i].propertyName + 
           "<br />");
     } 
    } else {
     document.writeln("No arrays. <br /><br />"); 
    }

    document.write('<h3>Non-array objects in this object:</h3>');
    if (objectPropertyList.length > 0) {
     var i;
     for (i = 0; i < objectPropertyList.length; i += 1) {
      document.writeln("<strong>Object name:</strong> " +
           objectPropertyList[i].propertyName + 
           "<br />");
     } 
    } else {
     document.writeln("No objects. <br /><br />"); 
    }

    document.write('<h3>Scalars for this object:</h3>');
    if (scalarPropertyList.length > 0) {
     var i;
     for (i = 0; i < scalarPropertyList.length; i += 1) {
      document.writeln("<strong>Name:</strong> " +
           scalarPropertyList[i].propertyName + 
           " | <strong>Value:</strong> " +
           scalarPropertyList[i].propertyValue +
           " | <strong>Data type:</strong> " +
           scalarPropertyList[i].propertyType +
           "<br />");
     } 
    } else {
     document.writeln("No scalar variables. <br /><br />"); 
    }

} // end function

And the new call:

<h2>DOM Element to look for:</h2>
<input id="inputElementToLookFor" type="text" value="monkeyShines"  />

<h2>A created object literal:</h2>
<script type="application/javascript">
// An object we created
var testObj = {
    'first-name' : "dan",
    'aNumber' : 234,
    'anArray' : [1,2,3,4],
    'anObject' : {"one":1, "two":2},
    'aMethod' : function() {}
};

printObjectReflection(testObj);
</script>

<h2>An HTML element we pulled from the DOM: </h2>
<script>
// An HTML element
var myInputElement = document.getElementById("inputElementToLookFor")
printObjectReflection(myInputElement);
</script>

Thanks everyone for the help. It still doesn't work in IE, sadly... if I come up with a final implementation that does, I will post it.

Answer 1

Use [] instead of Array() because Array can be overridden.

Answer 2

In your implementation this line causes an error in IE

myInputElement.printObjectReflection = printObjectReflection;

Answer 3

The code looks OK. If you really want to impress them, consider putting a result viewer together that presents the information in a list (or table), and allows objects and arrays to be clicked, whereupon they are displayed in a nested list (or table).

It may seem like overkill for an interview question, but it's actually fairly simple to do, and demonstrates a good understanding of the DOM.

More importantly, it provides something to prompt further questions from the interviewers. Rememeber: they will only have a certain amount of time, and this will keep their questions focused on a piece of code that you understand thoroughly, reducing the risk of them hitting you with a question about something you don't know so well.

The more you can get them to spend time talking about your own code, the more confident you'll feel, and the better your chances :-)

EDIT: following on from Chad's comment about the IE error, I'd argue against using the reflection approach of adding the function as a property of the object being examined and then iterating over this - apart from anything else, it means the function will show up as a property, and modifying the thing you're inspecting is a bad idea. Just pass the item as an argument to the function - might I respectfully suggest the name that for the argument.