Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I've been trying to create a tree module in JavaScript for implementing a "family tree" style structure. I'm not very experienced in newer JavaScript design styles and want to make sure I am going along the right path with the code I have written so far. Can someone let me know if I am on the right track with how this is organized?

var TREE = (function() {

var treeList = [], //Tree container

    //Constructor for Member object
    Member = function(name, mother, father, children){
        this.name = name || null;
        this.mother = mother || null;
        this.father = father || null;
        this.children = children || null;
    },

    //Constructor for Tree object
    Tree = function(name){
        this.name = name || null;
        this.memberList = [(new Member("root"))];
    };

//Tree methods  
Tree.prototype.addMember = function(name, mother, father, children){
    this.memberList.push(new Member(name, mother, father, children));
}
Tree.prototype.getMemberList = function(){
    return this.memberList;
}
return {
    create : function(treeName){
        treeList.push(new Tree(treeName));
        return treeList[treeList.length - 1];
    },
    getTreeList : function(){
        return treeList;
    }
};

}());
share|improve this question
    
I suggest you finish up the code first before we review it. That way, further optimizations can be done with your code. As far as I can see, this code should work as intended. –  Joseph the Dreamer Mar 26 '13 at 2:01

1 Answer 1

up vote 0 down vote accepted

Looks okay to me,

  • I would not expose function names mentioning node, but rather familyMember or member.

  • You expose only name through addNode, but you have name, mother, father, children in the constructor of Node.

  • Your var statement is not well indented, making the code harder to understand

  • I am curious as to treeList, what use case do you have in mind for that?

share|improve this answer
    
Thanks for the comments. I still have quite a few things to add to the module but I wanted to make sure it was structured correctly. For instance I wasn't sure whether the "Node" object should be accessible outside of the "Tree" context. For your question, the treeList is supposed to let me add and keep track of more than one tree if I required. –  William MacDonald Mar 26 '13 at 14:38

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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