Solved

Better way of using callback in javascript

Posted on 2016-11-01
7
81 Views
Last Modified: 2016-11-21
Hi,
Here is the code which i am using ... this includes a callback... although code seems to be working fine.. Need help from experts on its design a better way to write the same or any flaws in it  :
clock.groups.list(userId, callback);
clock.roster.listContacts(userId, callback);

var callback = function(errorThrown, data, name) {
         if(errorThrown === null) {
             if(name === 'groups.list') {
                 mygroupList = data;
             }
             else if (name === 'roster.listContacts') {
                 data.forEach(function(elem){mybuddyList.push(elem)});
             }
         }
}

exports.groups = {};
exports.groups.list = function(userId, callback) {
    callMethod('groupList', userId, callback);
}
exports.roster = {};
exports.roster.listContacts = function(userId, callback) {
    callMethod('listContacts', userId, callback);
}

callMethod = function(name, userId, callback) {
    $.ajax({
        url : endpoint,
        type: 'POST',
        contentType: 'application/json',
        headers: {
            'X-user-id': userId,
        },
        dataType: 'json',
        success: function(data, textStatus, jqXHR){
            callback(null, data, name);
        },
        error: function(jqXHR, textStatus, errorThrown) {
            callback(errorThrown, null, name);
        }
    });
}

Open in new window


Thanks
0
Comment
Question by:Rohit Bajaj
7 Comments
 
LVL 43

Expert Comment

by:Rob
ID: 41869411
Why would you want to do that if it's working... Not sure I understand?  Is it an assignment?
0
 
LVL 50

Assisted Solution

by:Steve Bink
Steve Bink earned 83 total points
ID: 41869548
I did something similar awhile ago.  I found that I had a need to run the same bit of code for each AJAX call (error checking, data prep, etc.), so I wrote a wrapper to go around it.  Take a look at doAjax() and handlerDoAjax().  This allows for my common code to be run after each call, and also a list of custom callbacks passed in by the caller.  You can find some brief examples of how I used it in other sections of that file, or some of the other JS files in the repo (e.g., CartPlugin.js, order-status.js).
0
 

Author Comment

by:Rohit Bajaj
ID: 41869590
no its not an assignment.. but its always good to look for alternatives and improve code quality... may be my code has some design flaws...for which i need advice from experts...
0
How our DevOps Teams Maximize Uptime

Our Dev teams are like yours. They’re continually cranking out code for new features/bugs fixes, testing, deploying, responding to production monitoring events and more. It’s complex. So, we thought you’d like to see what’s working for us. Read the use case whitepaper.

 
LVL 55

Assisted Solution

by:Julian Hansen
Julian Hansen earned 83 total points
ID: 41869696
Well the trend is to use promises rather than callbacks. To that end the use of the success and error properties in your $.ajax call should probably be changed as they are deprecated - rather use the .done()

$.ajax({
  url: ...,
  data: ...
}).done(function(resp) {
});

Open in new window

0
 
LVL 27

Assisted Solution

by:BigRat
BigRat earned 83 total points
ID: 41870007
The ajax call makes a distinction between success and failure by having the two cases define functions in the object with keys "success" and "error". Why then do you 1) define a function which simply calls another function - instead of doing the action directly and 2) call that function in the error case and then ignore the error if it occurs? Furthermore I see no point in the function (defined as a var) of a  callback which then makes a distinction as to what to do.

I'd simplify all of this into two callbacks - with distinctive names like "processGroups" and "processRoster" which assign the data to the corresponding variables and has a signature of "function(data, textStatus, jqXHR)". I'd simply pass these into the ajax call which sets the success field to that function.
callMethod = function(name, userId, callback) {
    $.ajax({
        url : endpoint,
        type: 'POST',
        contentType: 'application/json',
        headers: {
            'X-user-id': userId,
        },
        dataType: 'json',
        success: callback
        },
        error: function(jqXHR, textStatus, errorThrown) {
            // DO SOME ERROR ACTION HERE
        }
    });
}

Open in new window

0
 
LVL 82

Expert Comment

by:leakim971
ID: 41870039
what is clock.groups.list and clock.roster.listContacts?
0
 
LVL 82

Accepted Solution

by:
leakim971 earned 251 total points
ID: 41870041
my two cents depending the two hidden functions not showed by your code snippet :
var setGroupList = function(list) {
    mygroupList = list;
}

var setContactsList = function(lists) {
    lists.forEach(function(elem){
        mybuddyList.push(elem)
    });
}

// you should call this each time userId change
// so I'm pretty sure it's not the right place for it here
$.ajaxSetup({
    headers: { 'X-user-id': userId },
    error: handleError 
});

var handleError = function(jqXHR, textStatus, errorThrown) {
    // you should create something strong to help the end user instead the following
    console.log(errorThrown); // for debugging purpose, not for production    
}

clock.groups.list(userId, setGroupList);
clock.roster.listContacts(userId, setContactsList);

exports.groups = {
    list = function() {
        $.post(endpoint, function(data) { setGroupList(data); }, "json");
    }  
};

exports.roster = {
    listContacts: function(userId, callback) {
        $.post(endpoint, function(data) { setContactsList(data); }, "json");
    }
};

Open in new window

0

Featured Post

MIM Survival Guide for Service Desk Managers

Major incidents can send mastered service desk processes into disorder. Systems and tools produce the data needed to resolve these incidents, but your challenge is getting that information to the right people fast. Check out the Survival Guide and begin bringing order to chaos.

Question has a verified solution.

If you are experiencing a similar issue, please ask a related question

Suggested Solutions

Title # Comments Views Activity
Not allowed to load local recource. 4 35
parse url to form? 7 25
How  can  I extract  Id  from a  URL  using  Javascript? 12 29
message Alert on an empty search 10 22
Introduction A frequently asked question goes something like this:  "I am running a long process in the background and I want to alert my client when the process finishes.  How can I send a message to the browser?"  Unfortunately, the short answer …
Boost your ability to deliver ambitious and competitive web apps by choosing the right JavaScript framework to best suit your project’s needs.
The viewer will learn how to dynamically set the form action using jQuery.
The viewer will learn the basics of jQuery, including how to invoke it on a web page. Reference your jQuery libraries: (CODE) Include your new external js/jQuery file: (CODE) Write your first lines of code to setup your site for jQuery.: (CODE)

829 members asked questions and received personalized solutions in the past 7 days.

Join the community of 500,000 technology professionals and ask your questions.

Join & Ask a Question