-2
\$\begingroup\$

Below is the basic shell of an application I am working on that is part of a Webhosting Control Panel. This part is for the DNS records management.

So in my code below, I have taken away all the main functionality as it's not relevant to my question and to make it less cluttered.

On the page I have between 12-15 JavaScript Events ranging from click events to keypress event to keydown events etc...

Right now I have these main functions...

dnsRecords.init()    
dnsRecords.events.init()    
dnsRecords.records.addRow(row)    
dnsRecords.records.save()    
dnsRecords.records.undoRow(row)    
dnsRecords.records.deleteRow(row)

I have put all my Events into dnsRecords.events.init(). So each Event basically calls or passes Data to the dnsRecords.records functions.

Since I am new to JavaScript I am wanting to know if there is anything really wrong with this method or is there is a better location or way to put all those different Events?

Is it generally a good idea or not to put all my Events into 1 area like that and then have the fire of other functions instead of cluttering there callback area with logic code?

Also note, I am not looking to use Backbone or some other Framework at this time, just want to know a good way to structure a small single page application like this. Thank you.

var dnsRecords = {

    unsavedChanges : false,

    init: function() {
       dnsRecords.events.init();
    },

    events: {

        init: function() {

            // If user trys to leave the page with UN-SAVED changes, we will Alert them to...
            $(window).on('beforeunload',dnsRecords.events.promptBeforeClose);

            $(document).on('keypress','#typeMX div.hostName > input',function() {

                $(document).off('keypress','#typeMX div.hostName > input');
            });


            // Activate SAVE and UNDO Buttons when Record Row EDITED
            $(document).on("keydown", "#dnsRecords input" ,function() {

            });

            // Add new Record Row
            $("#dnsRecords div.add > .btn").click(function(e) {
                e.preventDefault();
            });

            // Mark Record as "Deleted" and change the view of it's Row to reflect a Deleted item
            $(document).on("click", ".delete" ,function(e) {
                dnsRecords.records.deleteRow($(this));
                e.preventDefault();
            });

            // Show Undo button when editing an EXISTING ROW
            $(document).on("keydown","div.dnsRecord input[type='text']",function() {

            });

            // Undo editing of an EXISTING ROW
            $("button.undo").on("click",function() {
                dnsRecords.records.undoRow($(this));
            });

            //Save Changes
            $("#dnsTitle a.save").click(function() {
                 zPanel.loader.showLoader();
              });

            //Undo ALL Record Type Changes
            $("#dnsTitle a.undo").click(function() {

            });

            $("form").submit(function() {

            });

        },

    },

    records: {

        addRow: function(record) {
            // All the code to Add a record here
        },

        save: function() {
            // All the code to Save a record here
        },

        undoRow: function(row) {
            // All the code to Undo a record here
        },

        deleteRow: function(row) {
            // All the code to Delete a record here
           dnsRecords.unsavedChanges = true;
        },

    }

};

$(function(){
   dnsRecords.init();
});
\$\endgroup\$
1
  • \$\begingroup\$ Is this code standalone? or are there other parts of your application using this dnsRecords stuff? which parts? \$\endgroup\$ – Joseph Apr 24 '13 at 5:26
1
\$\begingroup\$

General tips:

  • Use semi-colons. In JS, although they are optional, you should use them to avoid syntax errors, especially when minifying.

  • To modularize your code, wrap them in a closure. Consider it your "sandbox" for your code.

As for the rest, it's in the comments

//enclosing it in a closure so we won't spill to the global scope
//a general tip I keep is that 
//if this module performs something only it should do, then keep it in the scope (private)
//everything that others can use (public) gets exposed via the namespace
(function (window, document, $, undefined) {

    //we cache a few useful values, like jQuery wrapped window and document
  var $window = $(window),
    $document = $(document),

    //here's an example of exposing. we expose this object as dnsRecords
    //to the global scope. Since assignment operations "spill left"
    //the same object gets assigned to the local dnsRecord
    //we do this since every access to a property (something dot something)
    //is an overhead. also, assigning to a variable is shorter anyway.
    dnsRecords = window.dnsRecords = {
      unsavedChanges: false
      addRow: function (record) {},
      save: function () {},
      undoRow: function (row) {},
      deleteRow: function (row) {
        dnsRecords.unsavedChanges = true;
      }
    };

  //here, we declare your init methods
  //like stated above, since only this module uses init, it's kept in the scope
  //rather than it being exposed
  function bindDom() {

    //we then use the cached values
    $window.on('beforeunload', dnsRecords.promptBeforeClose);

    //did you know that the on method returns the object it operated on
    //which means it returns $document, which also means we can chain on
    //also, I suggest you delegate to the nearest available parent
    //for shorter delegation. In this code you have, the event needs
    //to "bubble" to the document root in order for handlers to execute
    //which is also an overhead, and the same reason live was deprecated

    $document
      .on('keypress', '#typeMX div.hostName > input', function () {
        $document.off('keypress', '#typeMX div.hostName > input');
      })
      .on("keydown", "#dnsRecords input", function () {

      })
      .on("click", ".delete", function (e) {
        dnsRecords.deleteRow($(this));
        e.preventDefault();
      })
      .on("keydown", "div.dnsRecord input[type='text']", function () {

      });

    $("#dnsRecords div.add > .btn").click(function (e) {
      e.preventDefault();
    });

    $("button.undo").on("click", function () {
      dnsRecords.undoRow($(this))
    });

    $("#dnsTitle a.save").click(function () {
      zPanel.loader.showLoader();
    });

    $("#dnsTitle a.undo").click(function () {});

    $("form").submit(function () {});
  }

  //you can declare other functions here as well to split operations
  function someOtherInitStuff(){
    ...
  }

  //I notice your init function is called on documentReady
  //why not merge it in the module, and make that function your init
  $(function () {

    //call stuff you want to init
    bindDom();
    someOtherInitStuff();
  });

}(this, document, jQuery));

//out here, your exposed methods are like:
dnsRecords.addRow();
dnsRecords.save();
\$\endgroup\$
2
  • \$\begingroup\$ Thanks for the demo, I have been reading more about thios style of "modules" today. As far as this particular code, it does not need to be accessed anywhere else and all my functions are called from the different Events on the page, so possibly everything could be private? I was looking at some of your other posts to, good stuff \$\endgroup\$ – JasonDavis Apr 24 '13 at 7:23
  • \$\begingroup\$ @jasondavis Yup, you can keep everything in the module. You can turn unsavedChanges into a local variable, and discard the window.dnsRecords. That way, dnsRecords is a collection of operations that is only accessible from the inside. \$\endgroup\$ – Joseph Apr 24 '13 at 7:27
3
\$\begingroup\$

What you are doing seems pretty much the standard way to do this sort of thing with Javascript and jQuery .

A few pointers:

Often, you wrap functions with other anonymous functions, for example:

$("#dnsTitle a.save").click(function() {
    zPanel.loader.showLoader();
});

Could likely become:

$("#dnsTitle a.save").click(zPanel.loader.showLoader)

Speaking of which, every time you have an anonymous function that handles considerable amount of code, you should consider extracting it to a named method.

Is it generally a good idea or not to put all my Events into 1 area like that and then have the fire of other functions instead of cluttering there callback area with logic code?

Yes, it is. Generally your event listeners when writing this sort of code 'control' the flow of events. That area (like in your code) is in charge of negotiating between your data and user/presentation, letting the logic layer know when the change happened. It is generally cleaner to register all such handlers in a specific place.

\$\endgroup\$
1
  • \$\begingroup\$ sounds like some good valid advice, thank you \$\endgroup\$ – JasonDavis Apr 24 '13 at 4:45

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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