JavaScript Code Organization Suggestions/Code Review

Question

1. I am working on a large site that has a lot of custom (page specific js). There is one main.js and page-specific.js files. Is there a better approach that can use the following pattern?

2. How can I re-factor multiple methods that use ajax?

3. I currently assign all onclick events inline such as onclick="MYSITE.message.send... - it there a better way? Creating multiple $("#button").click(function() {}); seems like more work...

   var MYSITE = MYSITE ? MYSITE: {};
   var MYSITE {  
   bookmark: {
       add: function(contentId, userId) {  
           var data = {  
               contentId: contentId,  
               userId: userId  
           };  
           $.ajax({  
               url: "/rest/bookmarks/",  
               type: "post",  
               data: data,  
               complete: function(response) {  
                   if (response.error) {  
                       alert(response.error);  
                   } else {  
                       alert("success");  
                   }  
               }  
           });  
       }  
   },  
   message: {  
       /* <a onclick="MYSITE.message.send('1234', '1234');" href="javascript:void(0);">BOOKMARK</a> */  
       send: function(contentId, userId) {  
           var data = {  
               contentId: contentId,  
               userId: userId  
           };  
           $.ajax({  
               url: "/rest/bookmarks/",  
               type: "post",  
               data: data,  
               complete: function(response) {  
                   if (response.error) {  
                       alert(response.error);  
                   } else {  
                       alert("success");  
                   }  
               }  
           });  
       }  
   }  
   

   }

Answer 1

Firstly, assigning onclick="" directly is bad practice. Using jQuery's click() method is not that much more work, but even if it was, it is cleaner and much more recommended. It keeps your HTML markup separate from your JS functionality and makes it easier to change things later on. That's the very first thing you should refactor.

What I do with my JS is create one main library JS file with all of my classes/core functionality. I then use inline <script> tags on specific pages to call the method that hooks up the links to the functions.

In the global app.js file, I'll have a function:

function initLinksOnPageX() {
  $('#button').click(function() { MYSITE.message.send('1234', '1234'); });
  /* ... setup any other events ... */
}

In the page body:

<script type="text/javascript">
  $(initLinksOnPageX);
</script>

Again, this keeps your markup clear of any JS code, so your JS related updates happen in one file (or fewer). With this setup you shouldn't strictly need page-specific JS files, though organizationally speaking it might make sense-- I don't know exactly how big your JS is.

Answer 2

You can use jquery delegate method if you have div element with a tags

<div id="bookmarks">
<a href="#">BOOKMARK</a>
</div>

$("div#bookmarks").delegate("a", "click", function(){
    MYSITE.message.send('1234', '1234');
});

and you can get 'contentId' and 'userId' dynamically.

Answer 3

My thoughts:

1. This depends on many factors, but if the page specific code is "large" then it is probably best to keep it separate. If, however, it is just a few lines, and we are not talking about thousands of pages, then lumping it all into the main.js may not be a big issue.

2. Not very familiar with ajax, so I will pass on commenting on how to "refactor".

3. If you can loop your assignment of the onclick events, then NOT doing them inline is going to save you a lot of work.

Answer 4

I'd consider wrapping my custom functionality up as a jquery plugin.