Что не так (или правильно) с этим шаблоном объекта JS?

Question

Вот пример шаблона, который я использую в своих объектах javascript в эти дни (в этом примере используется jQuery). http://pastie.org/private/ryn0m1gnjsxdos9onsyxg

Это работает для меня достаточно хорошо, но я предполагаю, что что-то не так или, по крайней мере, неоптимально, мне просто интересно узнать мнение людей.

Вот небольшой встроенный пример:

sample = function(attach) {
    // set internal reference to self
    var self = this;

    // public variable(s)
    self.iAmPublic = true;

    // private variable(s)
    var debug = false;
    var host = attach;
    var pane = {
        element: false,
        display: false
    }

    // public function(s)
    self.show = function() {
        if (!pane.display) {
            position();
            $(pane.element).show('fast');
            pane.display = true;
        }
    }

    self.hide = function() {
        if (pane.display) {
            $(pane.element).hide('fast');
            pane.display = false;
        }
    }

    // private function(s)
    function init () {
        // do whatever stuff is needed on instantiation of this object
        // like perhaps positioning a hidden div
        pane.element = document.createElement('div');
        return self;
    }

    function position() {
        var h = {
            'h': $(host).outerHeight(),
            'w': $(host).outerWidth(),
            'pos': $(host).offset()
        };
        var p = {
            'w': $(pane.element).outerWidth()
        };
        $(pane.element).css({
            top: h.pos.top + (h.h-1),
            left: h.pos.left + ((h.w - p.w) / 2)
        });
    }

    function log () {
        if (debug) { console.log(arguments); }
    }

    // on-instantiation let's set ourselves up
    return init();
}

Мне действительно любопытно узнать мнение людей по этому поводу.

Answer 1

sample = function(attach) {
    // set internal reference to self
    var self = this;

Почему вы не используете this напрямую? Напишите Javascript, тот, кто поддерживает ваш код, поблагодарит вас .

    // public variable(s)
    self.iAmPublic = true;

    // private variable(s)
    var debug = false;
    var host = attach;
    var pane = {
        element: false,
        display: false
    }

    // public function(s)
    self.show = function() {
        if (!pane.display) {
            position();
            $(pane.element).show('fast');
            pane.display = true;
        }
    }

    self.hide = function() {
        if (pane.display) {
            $(pane.element).hide('fast');
            pane.display = false;
        }
    }

Я знаю, что это хороший дизайн ООП, но на самом деле вы добавляете еще еще один слой косвенности к простому Javascript. У вас будут некоторые вещи, которые вы будете скрывать с помощью $(e).hide('fast'), и другие, которые вы будете скрывать с помощью e.hide(). Это несоответствие может сделать ваш код немного более запутанным, чем необходимо.

    // private function(s)
    function init () {
        // do whatever stuff is needed on instantiation of this object
        // like perhaps positioning a hidden div
        pane.element = document.createElement('div');
        return self;
    }

Подготовьте _ или __ перед закрытыми методами и переменными, поскольку Javascript не имеет закрытых атрибутов, поэтому вы должны полагаться на соглашение или использовать замыкания .

    function position() {
        var h = {
            'h': $(host).outerHeight(),
            'w': $(host).outerWidth(),
            'pos': $(host).offset()
        };
        var p = {
            'w': $(pane.element).outerWidth()
        };
        $(pane.element).css({
            top: h.pos.top + (h.h-1),
            left: h.pos.left + ((h.w - p.w) / 2)
        });
    }

    function log () {
        if (debug) { console.log(arguments); }
    }

Этот фрагмент кода немного неоптимален, так как функция log () будет вызываться извне класса. Я бы точно не оставил это в рабочем коде.

    // on-instantiation let's set ourselves up
    return init();
}