1

I'm redisigning an old script, making it OOP first and then later use jQuery for it. I know this question is asked before, but the answers don't seem to fit in my situation. Or I don't get it without jQuery knowledge.

This working script resizes a DIV based on mousevents, that pass a boolean property between them. But the methods reference properties by object name which is bad OOP I understand. I see a lot of answers with a "init" function and renaming "this" that I understand, but I can't relate those solutions to my script:

See my Fiddle.

PositionXY = {
    baseX : undefined,
    baseY : undefined,
    isRepositionable : true,

    detectBasePosition : function (e) {

        "use strict";
        PositionXY.isRepositionable = true;   // bad OOP
        e = e || window.event;    // crossbrowser event acces
        if (this.currentStyle){   // IE
            if (e.clientX || e.clientY) {
                PositionXY.baseX = e.clientX - this.currentStyle.width.replace(/px/,"");
                PositionXY.baseY = e.clientY - this.currentStyle.height.replace(/px/,"");   
            }
        } else if (window.getComputedStyle){  // Mozilla
            if (e.pageX || e.pageY) {
                PositionXY.baseX = e.pageX - document.defaultView.getComputedStyle(this,null).getPropertyValue("width").replace(/px/,"");
                PositionXY.baseY = e.pageY - document.defaultView.getComputedStyle(this,null).getPropertyValue("height").replace(/px/,"");
            }
        }
    },
    startPositionAdjustment : function (e) {

        "use strict";
        if (PositionXY.isRepositionable) {   // bad OOP
            e = e || window.event;           // crossbrowser event acces
            if (e.clientX || e.clientY) {    // IE
                this.style.width = e.clientX - PositionXY.baseX + "px";
                this.style.height = e.clientY - PositionXY.baseY + "px";
            } else if (e.pageX || e.pageY) {  // Mozilla 
                this.style.width = e.pageX - PositionXY.baseX + "px";
                this.style.height  = e.pageY - PositionXY.baseY + "px";
            }
        } else return;
    },
    stopPositionAdjustment : function () {

        "use strict";
        PositionXY.isRepositionable = false;    // bad OOP
    }
};


var elm = document.getElementById("x");
elm.onmousedown = PositionXY.detectBasePosition;
elm.onmousemove = PositionXY.startPositionAdjustment;
elm.onmouseup = PositionXY.stopPositionAdjustment;
oysterhoys
  • 65
  • 1
  • 7
  • 1
    You haven't asked a question – Jamiec Feb 02 '15 at 09:45
  • 1
    No, it's not bad OOP. At least if you don't need multiple instances of your object. [It's even kinda required for your code](http://stackoverflow.com/q/10711064/1048572), using `this` would be an unnecessary hazzle. – Bergi Feb 02 '15 at 09:45
  • Sorry, could be more clear. The question is: is there ( and should there, as Bergi says it is actually not a problem) a better way to reference the the property "isRepositionable" then by "PositionXY.isRepositionable" , like by some way of "internal" referencing ? – oysterhoys Feb 02 '15 at 10:03
  • Sure, you can make it a private state of the object. The easiest way to do this is to build the public methods in some `init` function, creating a closure over local variables instead of using the fields. It's a commonly used pattern in modern Javascript. – Luaan Feb 02 '15 at 11:43
  • What's wrong with `this.isRepositionable`? – plalx Feb 02 '15 at 13:46
  • @plalx `this` would refer to the DIV the event was triggerend on – oysterhoys Feb 02 '15 at 13:56
  • @Luaan: I've seen these `init` solutions, but I don't see how to implement them in this situation. Perhaps you could sketch it, or point to a similar question ? – oysterhoys Feb 02 '15 at 13:59
  • @oysterhoys That's because of the way you are binding your handlers. See my answer. – plalx Feb 02 '15 at 14:16

1 Answers1

1

You could use this within the functions, but you would have to make sure to properly bind the event handlers:

 var elm = document.getElementById("x");
 elm.onmousedown = PositionXY.detectBasePosition.bind(PositionXY);
 elm.onmousemove = PositionXY.startPositionAdjustment.bind(PositionXY);
 elm.onmouseup = PositionXY.stopPositionAdjustment.bind(PositionXY);

However, the only advantage you gain here by using this is the ability to re-alias PositionXY to some other variable while changing the initial value of PositionXY, which will probably never occur.

Using an IIFE, you can also overcome that issue while keeping your initial design's simplicity:

var YourObject = (function () {
    var YourObject = {
        someProperty: 'test',
        someFunction: function () {
            console.log(YourObject.someProperty);
        }
    };

    return YourObject;
})();
plalx
  • 42,889
  • 6
  • 74
  • 90
  • That constructor is not even necessary if the object should stay a singleton, but a module IEFE would fit just as well. – Bergi Feb 02 '15 at 14:23
  • @Bergi No the constructor is not necessary but I use it as an IIFE here. Otherwise you would achieve the same... `PositionXY = (function () { var o = { fn: function () {//use o} }; return o; })();` – plalx Feb 02 '15 at 14:25
  • oh I see now, you're overwriting the constructor with an instance. But notice that [abusing `new` for IEFEs is an antipattern](http://stackoverflow.com/q/10406552/1048572) – Bergi Feb 02 '15 at 14:28
  • @Bergi Right, there would be constructor leakage with that approach. While responding to your comment I realized that using an IIFE without using `this` is in fact a much better approach in this case. – plalx Feb 02 '15 at 14:43
  • Thanx Guys, as a JS OOP newbee this gives me some food for thought. – oysterhoys Feb 02 '15 at 16:49