HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

Functional organisation of JavaScript functions

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
functionalfunctionsorganisationjavascript

Problem

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),

Solution

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...

Code Snippets

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();
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);
    }
};

Context

StackExchange Code Review Q#72174, answer score: 2

Revisions (0)

No revisions yet.