current community

your communities

more stack exchange communities

Stack Exchange
tour help
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.
up vote 1 down vote favorite
1

The last month I've been reading up on how to take my JavaScript code to the next level. I am a Java programmer at heart, so it feels nice when everything is encapsulated in classes/objects, and these are ready to be instantiated or inhertid or whatever.

I wrote a simple test project with the purpose to refine my techniques, and I'm feeling I can't do anything more with it. Would you guys and gals take a look at it?

var People = People || {};

People = (function() {
    function People() {
        this.people = [];
    }

    People.prototype.addGentleman = function (name) {
        this.people.push(new People.Gentleman(name));
    }

    People.prototype.addLady = function (name, isMarried) {
        this.people.push(new People.Lady(name, isMarried));
    }

    People.prototype.getList = function () {
        var temp = [];

        for (var i = 0; i < this.people.length; i++) {
            temp.push(this.people[i].toString());
        }

        return temp;
    }

    return People;
})();

People.Human = (function () {
    function Human(name, hasLadyParts) {
        this.name = name;
        this.hasLadyParts = hasLadyParts;
    }

    Human.prototype.hasLadyParts = function () {
        return this.hasLadyParts;
    }

    Human.prototype.toString = function () {
        var str = this.name + ' has ';
        if (!this.hasLadyParts) str += 'no ';
        return str + 'lady parts.';
    }

    return Human;
})();

People.Gentleman = (function () {
    function Gentleman (name) {
        People.Human.call(this, name, false);
    }

    Gentleman.prototype = Object.create(People.Human.prototype);

    Gentleman.prototype.toString = function () {
        return 'Mr. ' + People.Human.prototype.toString.call(this);
    }

    return Gentleman;
})();

People.Lady = (function () {
    function Lady(name, hasWeddingRing) {
        People.Human.call(this, name, true);
        this.hasWeddingRing = hasWeddingRing;
    }

    function isNotMarried() {
        return !this.hasWeddingRing;
    }

    Lady.prototype = Object.create(People.Human.prototype);

    Lady.prototype.toString = function () {
        var str = !isNotMarried.call(this) ? 'Mrs. ' : 'Ms. ';
        return str + People.Human.prototype.toString.call(this);
    }

    return Lady;
})();

$(function () {
    var people = new People();
    people.addGentleman('Viktor');
    people.addGentleman('Joakim');
    people.addLady('Mattias', true);
    var list = people.getList();
    var $ul = $('#people');

    for (var i = 0; i < list.length; i++) {
        $ul.append('<li>' + list[i] + '</li>');
    }
});

One question: can I create protected (or public) variables that my child instance can change? Besides that question I only want my code reviewed with regards to my Java context. It feels nice to have it like this, but what can I do better?

The code should be a pretty straight-forward OOP example, and here's a fiddle.

share|improve this question

2 Answers 2

up vote 1 down vote 
<!DOCTYPE html>
<html charset="utf-8">

    <head>
    </head>

    <body>

        <ul id=people></ul>

        <script>

            (function () { // create a new scope

                'use strict';
                /*jslint browser: true*/

                /*
                    My main advice is this: don't over-engineer. Even
                    something as basic as a class in Java is often an
                    unnecessary complication in JavaScript. Trying to
                    emulate the Java style can end up messy and
                    contrived.

                    Modern JavaScript is more likely to have leanings
                    towards a functional style, so you should be
                    comfortable with that.

                    This and both the expressive syntax for object
                    literals and their malleability during run-time get
                    you most of what you need.

                    Inheritance hierarchies can be used for the
                    specialised parts of programs that require them, but
                    they are often not needed and certainly do not form
                    the basic architecture of a JavaScript program.

                    And if you do need an inheritance hierarchy keep it
                    shallow.

                    But you probably don't need one.
                */

                // Table of Contents
                var makeperson,
                    people,
                    listitems,
                    list;

                /*
                    Here I'm just returning an object constructed using
                    literal notation.
                */

                makeperson = function (title, name, gender, maritalstatus) {
                    return {
                        title: title,
                        name: name,
                        gender: gender,
                        maritalstatus: maritalstatus,
                        toString: function () {
                            return this.title + ' ' + this.name +
                                    ' (' + this.gender + ')';
                        }
                    };
                };

                /*
                    However in this case you only actually need

                    makeperson = function (title, name, gender) {
                        return this.title + ' ' + this.name +
                                ' (' + this.gender + ')';
                    };

                    but I thought I'd throw an object with a few
                    properties your way, just for illustration.

                    There's also no need to create a type to represent
                    a list of people. Variables are untyped in
                    JavaScript, so a plain array will do nicely. Array
                    instances come with lots of useful methods, e.g.
                    map, reduce, filter.
                */

                people = [
                    makeperson('Mr', 'Victor', 'm'),
                    makeperson('Mr', 'Joakim', 'm'),
                    makeperson('Mrs', 'Mattias', 'f', 'married')
                ];

                /*
                    And there's no need for a big library for a few
                    simple DOM manipulations.
                */

                listitems = people.map(function (person) {
                    var item = document.createElement('li');
                    item.appendChild(document.createTextNode(person));
                    return item;
                });

                list = document.getElementById('people');

                listitems.forEach(function (item) {
                    list.appendChild(item);
                });

            }());

            /*
                P.S.

                And make sure you use a linter and get into good habits
                early on.
            */

        </script>
    </body>
</html>
share|improve this answer
    
Although your constructor function does the trick: the result is "Object" instead of "Person". So in my opinion, it would be better to use a proper constructor function, which returns a "Person". cf. jsfiddle.net/hmAvw –  Thomas Junk May 10 '13 at 20:42
    
@Lilith2k3 en.wikipedia.org/wiki/Duck_typing –  mintsauce May 10 '13 at 20:52
    
D'accord - if it doesn't matter, whether you need a Duck or something that quacks ;) –  Thomas Junk May 10 '13 at 21:55
up vote 0 down vote

I'm not a JavaScript expert or highly experienced but still, here is my approach:

object creation:

// Class creation
function Vehicle(p) {
  this.brand = p.brand || "";
    this.model = p.model || "";
    this.wheels = p.wheels || 0;
}

// Main class' methods
Vehicle.prototype.getBrand = function () {
    return this.brand;
};
Vehicle.prototype.getModel = function () {
    return this.model;
};
Vehicle.prototype.getWheels = function () {
    return this.wheels;
};


// object creation
var myVehicle = new Vehicle({
    brand: "Mazda",
    model: "RX8",
    wheels: 4
});

// usage
console.log(myVehicle);

basic inheritance:

// Main class
function Vehicle(p) {
    this.brand = p.brand || "";
    this.model = p.model || "";
    this.wheels = p.wheels || 0;
}

// Main class' methods
Vehicle.prototype.getBrand = function() {
    return this.brand;
};
Vehicle.prototype.getModel = function() {
    return this.model;
};
Vehicle.prototype.getWheels = function() {
    return this.wheels;
};

// child class
function Car() {
    // inheritance initiation
    Vehicle.apply(this, arguments);
    // property overriding
    this.wheels = 4;
}

// prototype linking
// Here I'm checking If the browser supports ES5's Object.create
if (Object.create) {
    Car.prototype = Object.create(Vehicle.prototype);
} else {
// else I'm using a more "hacky" way of implementing Object.create's funtionality
    var temp = function() {};
    temp.prototype = Vehicle.prototype;
    Car.prototype = temp.prototype;
}

// method overriding
Car.prototype.getWheels = function() {
    return 4;
};


// object creation
var myCar = new Car({
    brand: "Mazda",
    model: "RX8"
});

// usage
console.log(myCar);
share|improve this answer

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.