3
\$\begingroup\$

In one of the web apps I contribute to, the main.js file has multiple interacting "objects". Unfortunately, I'm so bad at OOP in JavaScript, despite studying other questions, that I just have a bunch of functions divided by a comment block as follows for the editor "object":

//*****************
// Setup the editor
//*****************
//Functions for interaction with editor
var aceRange = ace.require('ace/range').Range;
var editor = null;
var marker = null;
var gui_updating = false;

function removeMarker() {
    if (marker != null) {
        editor.getSession().removeMarker(marker);
        marker = null;
    }
}

function annotateLine(d) { //Called on mouseover in graph
    removeMarker();
    marker = editor.getSession()
        .addMarker(new aceRange(d.line, 0, d.line, 10), 
            'highlight', 'fullLine', true);

    editor.getSession().setAnnotations([{row: d.line, type: 'info'}]);
}

function clearAnnotation(d) { //Called on mouseout in graph
    removeMarker();
    editor.getSession().clearAnnotations();
}

The initialization of the editor "object" and it's bonding to events is called later, but isn't included here as I don't think it will help.

This seems bad, but I can't figure out how to improve it other than dividing the objects into separate files. I also think it's important to divide the code functionally, but I'm not sure how to do that. Should I wrap the whole thing up in a function?

Like so:

var app_editor = function(ace, ace_editor){
    self.aceRange = ace.require('ace/range').Range;
    self.editor = ace_editor;
    self.marker = null;

    removeMarker = function() {
        if (self.marker != null) {
            self.editor.getSession().removeMarker(self.marker);
            self.marker = null;
        }
    }

    annotateLine = function(d) { //Called on mouseover in graph
        removeMarker();
        self.marker = self.editor.getSession()
            .addMarker(new self.aceRange(d.line, 0, d.line, 10), 
                'highlight', 'fullLine', true);

        self.editor.getSession().setAnnotations([{row: d.line, type: 'info'}]);
    }

    clearAnnotation = function(d) { //Called on mouseout in graph
        removeMarker();
        self.editor.getSession().clearAnnotations();
    }
};

Should I be using prototypes as well, even if there will only be one function ever? I understand what a closure is, but I still don't know how to apply it and if it's appropriate here. Alternatively, since my code has gotten so big, should I look into using a library?

This project is still in its early stages, so I'm open to radical changes.

\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

Should I be using prototypes as well, even if there will only be one function ever?

No, you will only benefit from using the prototype when you create a lot of instances of a constructor function. I won't go in to the details of that, since it has already been discussed a lot in various SO posts.

Without more context, I suggest that you create a revealing module:

var app_editor = (function(window, ace, ace_editor, undefined){

    var aceRange, editor, marker, session;

    function init(){
        aceRange = ace.require('ace/range').Range;
        editor = ace_editor;
        marker = null;
        session = editor && editor.getSession();
    }

    function removeMarker () {
        if (marker && session) {
            session.removeMarker(marker);
            marker = null;
        }
    }

    return {
        init: init,
        removeMarker: removeMarker 
    }

})(window, ace, ace_editor);

app_editor.init();

Now you can keep variables and methods private within your app_editor and only expose what you need to access from the outside. This is just a small sample to get you started.

Are ace and ace_editor already defined in the global namespace? If not, you probably want to pass them as arguments to the init methods. It's hard to say from your two examples.

Some other comments:

  • annotateLine = function(d) { - What's d? Use a more descriptive name.
  • You're chaining methods a lot, e.g. editor.getSession().clearAnnotations();. What if getSession() returns undefined? Do more checks rather than making assumptions.

Let me know if you want me to elaborate anything.

Update

Simple object literal example:

var app_editor = {

    aceRange: null,
    editor: null,

    init: function(aceRange, editor){
        this.aceRange = aceRange;
        this.editor = editor;
    },

    removeMarker: function(marker){
        if(!this.editor || !this.editor.session) return;
        this.editor.session.removeMarker(marker);
    }
};

You get the idea...

\$\endgroup\$
3
  • \$\begingroup\$ Thanks for the input. From what I understand, what the module pattern accomplishes mostly is defining which functions of the "object" are private and which are public. However, if this distinction isn't useful because all of my functions of my "object" are public, should I still use this pattern? In that case, using this pattern seems a bit WET, since it involves me re-typing all of my function names. Is there another pattern to use? \$\endgroup\$
    – Seanny123
    Dec 9, 2014 at 21:38
  • \$\begingroup\$ @Seanny123 You can always start with an object literal. If you find yourself in a position later on where you want to keep some of its properties private; make a module out of it. I can update with a small sample \$\endgroup\$
    – Johan
    Dec 9, 2014 at 21:42
  • 1
    \$\begingroup\$ Ah. I was avoiding the object literals because they seemed messy, but I see now that this was very silly and despite their simple structure they can greatly help with code readability and organisation. \$\endgroup\$
    – Seanny123
    Dec 9, 2014 at 21:55

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.