Question

I have a script that I'd like to seperate out to multiple modules. For example, putting all my mouse events in another module (mouseup, mousedown, etc).

I use a bunch of global variables to keep track of all of my objects on the screen. The mouse module will need to access these variables and alter them. I realize I can reveal each variable needed to other modules, but they are unable to alter them.

How is this best achieved using Javascript? I current use the revealing module pattern, and I read that I have to use getters and setters (such as used in C++ from what I recall from school), but I have quite a few global variables to keep track of. I'd need quite a few of these methods. Is this the way I should go about doing so?

Or... is this a case that shows me that an issue describes bad design.

I'm using Fabric.js to do the drawing and all that stuff.

drawscript.js

var DrawScript = function(){

    var DRAW = 0,
        SELECT = 1,
        SCROLL = 2;

    var MOUSEBUTTONS = {
        'S': 83,
        'C': 67,
        'D': 68,
        'DELETE': 46,
        'BACKSPACE': 8
    };

    var canvas;
    var _started       = false;
    var _x, _y;
    var square;
    var edgedetection  = 10; // pixels to snap
    var lines          = {};
    var lines_group;
    var rectangles_group;
    var rectangle_txt  = {};
    var _text_offset_x = 20;
    var _text_offset_y = 20;
    var pressedKeys    = [];

    var _stroke_colour_var    = "red";
    var _stroke_width_var     = 5;
    var _selectionColor       = "rgba(255,255,255,0.4)";
    var _selectionBorderColor = "rgba(0,0,0,0.5)";

    var selectedObjStartSettings = {};
    var resetOpacityOfObject     = false;
    var drawingMode              = true;
    var mouseGlobal              = null;
    var garbage                  = null;

    var window_height        = 0;
    var window_width         = 0;
    var canvasHeight         = 0;
    var canvasWidth          = 0;
    var bg_height            = 0;
    var _canvas_position = {
        'x': {},
        'y': {}
    };
    var _canvas_mouse_pos_x = 0;
    var _canvas_mouse_pos_y = 0;

    var _fullSizeOn      = false;
    var _fullWidth       = 0;
    var _fullHeight      = 0;
    var _fitWidth        = 0;
    var _fitHeight       = 0;
    var _scaleSizeWidth  = 0;
    var _scaleSizeHeight = 0;

    var setCanvasSize = function(){
        // stuff
    },

    checkDrawingMode = function(){
        // stuff
    },

    checkKeyDown = function(e){
        // stuff
    },

    saveDrawing = function(){
        // stuff
    },

    deleteObjectFromCanvas = function (obj){
        // stuff
    },

    setObjectStartPoints = function(){
        // stuff
    },

    drawNewRect = function(top, left, width, height, name){
        // stuff
    },

    updateCoords = function(x, y){
        // stuff
    },

    changeZoom = function(input){
        // stuff
    },

    init = function(){

        garbage = $("#garbage");

        // ===================================
        //  Set drawing mode when changed
        // ===================================
        $("input[name='draw']").change(function(){
            checkDrawingMode();
        });

        // ===================================
        //  Zoom On
        // ===================================
        $("input[name='zoom-on']").change(function(){
            changeZoom(this);
        });

        // ===================================
        //  Check keypress
        // ===================================
        $(document).keydown(function(e){
            checkKeyDown(e);
        });

        $(document).keyup(function(e){
            e.preventDefault();

            var key;

            if(e.metaKey)
            {
                // key = 91;
                return;
            }
            else
                key = e.keyCode;
            pressedKeys[key] = false;

            // console.log(pressedKeys);
        });

        // ===================================
        //  Save drawing
        // ===================================
        $("#save").click(function(){
            saveDrawing();
        });

        $(window).resize(function(){
            setCanvasSize();
        });

        // ===================================
        //  Create canvas and check drawing mode when launched
        // ===================================
        canvas = new fabric.Canvas('demoCanvas');
        setCanvasSize();
        checkDrawingMode();
        canvas.selectionLineWidth = 2;
        canvas.selectionDashArray = [4, 4];
        canvas.cornerColor        = "#900";
        canvas.hoverCursor        = 'pointer';
        canvas.moveCursor         = 'pointer';
        canvas.selection          = false;

        // ===================================
        //  Add groups
        // ===================================
        rectangles_group = new fabric.Group();
        lines_group = new fabric.Group();
        canvas.add(lines_group);

        // ===================================
        //  Get mouse events
        // ===================================
        canvas.on('mouse:down', function(options) { mousedown(options); });
        canvas.on('mouse:move', function(options) { mousemove(options); });
        canvas.on('mouse:up', function(options) { mouseup(options); });
        canvas.on('object:scaling', function(e) { e.target.resizeToScale(rectangle_txt, _text_offset_x, _text_offset_y); });
        canvas.on('object:selected', function() { setObjectStartPoints(); });
        canvas.on('object:moving', function(options) { objectMoving(options); });
    };

    return {
        init: init
    };

}();

I've removed the function implementations at the top for brevity-sake. Also, I definately need to make my variable names more uniform in terms of naming conventions. This is all pretty early-on so I apologize if this makes anyone want to bust my knee-caps!

I'm not sure how to split up the mouse events so my drawScript.js file doesn't become 3k lines. The mouse events all use the various groups and coordinate variable I've set up. Also, I need to share the canvas amongst the scripts that the app uses.

Maybe I'm just trying to hard and over-organzing, but I figured asking for help would help me become better at larger projects either way :)

Was it helpful?

Solution

Don't use the Revealing Module Pattern. Many people learn the Revealing Module Pattern from Addy Osmani's book without paying attention to Addy's warning.

As you've found out, when you reveal your closured variables to other modules, they cannot be changed. That's a fundamental weakness of the Revealing Module Pattern. You probably did something like this

var mouse = (function(){
  var x = 10, y = 20;
  var drawLineTo = function(x1,y1){
     console.log("Drawing line from " + x + "," + y + " to " + x1 + "," + y1);
  };
  return {
     x: x,
     y: y,
     draw: drawLineTo
  }

})

mouse.x = 50
mouse.draw(15,15) // print "Drawing line from 10,20 to 15,15"

and found that changing mouse.x doesn't change the value of x in the closure. While you can get around this by defining explicit getters and setters like so

var mouse = (function(){
  var x = 10, y = 20;
  var getX = function() {return x};
  var setX = function(val) {x = val};
  var drawLineTo = function(x1,y1){
     console.log("Drawing line from " + x + "," + y + " to " + x1 + "," + y1);
  };
  return {
     getX : getX,
     setX : setX,
     y: y,
     draw: drawLineTo
  }

})

mouse.setX(50);
mouse.draw(15,15) // print "Drawing line from 10,50 to 15,15"

That's just a stupid price to pay for using the Revealing Module Pattern. Much simpler is

var mouse = (function(){
  // put your private variables and methods here.
  return {
     x : 10,
     y: 20,
     draw: function(x1,y1){
        console.log("Drawing line from " + this.x + "," + this.y + " to " + x1 + "," + y1)
     }
  }

})

mouse.x = 50;
mouse.draw(15,15) // print "Drawing line from 50,20 to 15,15"

The key is to never reveal -- only put the things you intend to hide into the closure. Things intended to be public should be created as such.

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top