0

I developed this short script but I'm wondering what is the best way to make $ul, $el and some functions e.g. select private. At the moment these are part of public interface but I would like to hide these. Wrap into another function returning DropDown object maybe? What would be the proper way to do this? Code below:

namespace = {};

namespace.DropDown = function(el) {

    this.$el = $(el)
    this.$trigger = this.$el.find(".trigger");
    this.$ul = this.$el.find("ul");
    this.$li = this.$ul.find("li");
    this.$trigger.text(this.$ul.find(".selected").text());

    this.$trigger.on("click", $.proxy(this.open, this));
}

namespace.DropDown.prototype.open = function(e) {

    e.stopImmediatePropagation();

    this.$ul.addClass("open");

    // position selected element in the middle

    var scrollUp,
        panelCenter = this.$ul.scrollTop() + (this.$ul.innerHeight() / 2),
        selectedPositionTop = this.$ul.scrollTop() + this.$ul.find(".selected").position().top;

    if (selectedPositionTop > panelCenter) {
        scrollUp = selectedPositionTop - panelCenter;
        this.$ul[0].scrollTop = this.$ul[0].scrollTop + scrollUp;
    } else {
        scrollUp = panelCenter - selectedPositionTop;
        this.$ul[0].scrollTop = this.$ul[0].scrollTop - scrollUp;
    }

    // position elements whole container (list container)

    var triggerTop = this.$trigger.offset().top + (parseInt(this.$trigger.css("padding-top")) || 0) + (parseInt(this.$trigger.css("border-top") || 0)),
        t = Math.abs(triggerTop - this.$ul.find(".selected").offset().top);

this.$ul.css("top", -t + "px");
    this.$li.one("click", $.proxy(this.select, this));

    $(document).one("click", $.proxy(this.close, this));
}

namespace.DropDown.prototype.close = function() {

    this.$li.off("click");

    this.$ul.removeClass("open");

    this.$ul.css("top", "0px");

}
namespace.DropDown.prototype.select = function(e) {

    $(document).off("click");

    e.stopImmediatePropagation();

    this.$li.removeClass("selected");
    $(e.target).addClass("selected");

    this.$trigger.text(this.$ul.find(".selected").text());

    this.close(e);
}

$(function() {
    new namespace.DropDown($(".dropdown")[0]);
    new namespace.DropDown($(".dropdown")[1]);
    new namespace.DropDown($(".dropdown")[2]);
});

EDIT:

I managed to wrap it into another function so I could get the properties and functions e.g. $ul, select off the protootype and make private via closure. It does work and I am able to keep guts of the object private but is this the best way to got? it seems overly complicated to me. Also I'm sure I'm not the first to come up with this and so is there a name for this pattern? Modified code below:

namespace = {};

namespace.DropDown = function(el) {

    var $el = $(el),
        $trigger = $el.find(".trigger"),
        $ul = $el.find("ul"),
        $li = $ul.find("li");

    DropDown = function() {
        $trigger.text($ul.find(".selected").text());

        $trigger.on("click", $.proxy(this.open, this));
    }

    function select(e) {

        $(document).off("click");

        e.stopImmediatePropagation();

        $li.removeClass("selected");
        $(e.target).addClass("selected");

        $trigger.text($ul.find(".selected").text());

        this.close(e);
    }

    DropDown.prototype.open = function(e) {

        e.stopImmediatePropagation();

        $ul.addClass("open");

        // position selected element in the middle

        var scrollUp,
            panelCenter = $ul.scrollTop() + ($ul.innerHeight() / 2),
            selectedPositionTop = $ul.scrollTop() + $ul.find(".selected").position().top; //- $ul.find(".selected").outerHeight();

        if (selectedPositionTop > panelCenter) {
            scrollUp = selectedPositionTop - panelCenter;
            $ul[0].scrollTop = $ul[0].scrollTop + scrollUp;
        } else {
            scrollUp = panelCenter - selectedPositionTop;
            $ul[0].scrollTop = $ul[0].scrollTop - scrollUp;
        }

        // position elements whole container (list container)

        var triggerTop = $trigger.offset().top + (parseInt($trigger.css("padding-top")) || 0) + (parseInt($trigger.css("border-top") || 0)),
            t = Math.abs(triggerTop - $ul.find(".selected").offset().top);
        $ul.css("top", -t + "px");
        $li.one("click", $.proxy(select, this));

        //$ul[0].scrollTop = this.scrollPos;
        $(document).one("click", $.proxy(this.close, this));
    }

    DropDown.prototype.close = function() {

        $li.off("click");

        $ul.removeClass("open");

        $ul.css("top", "0px");
    }

    return new DropDown();
};

$(function() {
    new namespace.DropDown($(".dropdown")[0]);
    new namespace.DropDown($(".dropdown")[1]);
    new namespace.DropDown($(".dropdown")[2]);
});

EDIT 2:

I should have mentioed that I want to keep the methods on prototype rather than directly on the object itself via this.functionname. I want to avoid method duplication as this will happen if these are attached to this directly. For this simple reason this question is not duplicate.

spirytus
  • 10,726
  • 14
  • 61
  • 75
  • 1
    possible duplicate of [JavaScript private methods](http://stackoverflow.com/questions/55611/javascript-private-methods) – NanoWizard Oct 25 '14 at 22:31
  • @NanoWizard I don't think its a duplicate, please see my EDIT2 – spirytus Oct 25 '14 at 22:50
  • Keep in mind that your edit/proposed solution recreates the object constructor and its prototype functions everytime you instantiate a new one. I'm guessing this isn't what you want - you might as well just do `this.functionname = function ....`. Also, watch out - you made `DropDown` a global. This will create bugs because your instances may now share mutual state that you didn't intend. – goat Oct 25 '14 at 23:06
  • @goat excellent points and thanks for your comment. I knew I missed something .. and global, ouch.. Cant believe i mossed it.Anyway I can't think of any way of achieving it without attaching everything to "this" on every instance. Is there any better way to do it? – spirytus Oct 26 '14 at 00:26
  • What do you mean by "method duplication"? – Bergi Oct 26 '14 at 21:21
  • @Bergi, I mean attaching functions to each object instance via "this" directly rather than on prototype. – spirytus Oct 27 '14 at 00:51
  • @spirytus: There's nothing wrong with that (and you're kinda doing it already by creating those event listeners). But notice that a prototype method, which is *shared amongst all instances*, must always be public. – Bergi Oct 27 '14 at 00:57
  • @Bergi I would love to know your opinion.. in general do you reckon its ok to put functions on "this" as in this.select=function(){} rather than on prototype even though these will be instantiated for every object instance? Isn't putting functions on prototype more efficient when the objects are instantiated? I'm guessing in some (very rare) cases this might make a difference but if I don't need to instantiate thousands and thousands of object would attaching functions directly to "this" be ok? – spirytus Oct 28 '14 at 11:12
  • Yes, exactly. There are some cases where it's simply necessary, basically whenever you need a closure over the instance you need to create instance-specific functions. And in your case performance hardly matters, as - I hope - you don't have thousands of dropdowns on your page. – Bergi Oct 28 '14 at 13:11

3 Answers3

0

You can create private methods and variables via closures, however another approach which may be simpler and create more fluid / cleaner reading code could be to designate private/public via convention. For example:

this._privateVariable = true;
this.publicVariable = true;

using underscores for private, no underscore for public etc.. /edited thanks HMR

roger
  • 1,091
  • 1
  • 7
  • 15
  • You mean this._private and this.public I think. Using var suggests closure availability. – HMR Oct 26 '14 at 00:50
0

This is the general form for making public and private attributes

var MyClass = (function(){
    var myclass = function(){}
    var privateMethod = function(){ return 1; }
    myclass.prototype.publicMethod = function(){ return privateMethod() + 1; }

    return myclass;

})();

Properties that are put on myclass, or on myclass.prototype, will public. However, variables that are declared with var will not be publicly available, though other methods will still have access to them. In the above example, publicMethod is the only publicly available method, but it is able to call privateMethod, because they were defined in the same scope.

Raphael Serota
  • 2,157
  • 10
  • 17
0

like this also:

function SampleClass() {
  // private
  var _type = 'aClass';

  // private
  var _dothis = function (action) {
    return 'i did this ' + action;
  };

  return {
    // public
    type: _type,
    // public
    dothis: function (action) {
        return _dothis(action);
    }
  };
}
Coldstar
  • 1,324
  • 1
  • 12
  • 32