Improve company productivity with a Business Account.Sign Up

x
?
Solved

Better way of using callback in javascript

Posted on 2016-11-01
7
Medium Priority
?
120 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 51

Assisted Solution

by:Steve Bink
Steve Bink earned 332 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
Free Tool: Path Explorer

An intuitive utility to help find the CSS path to UI elements on a webpage. These paths are used frequently in a variety of front-end development and QA automation tasks.

One of a set of tools we're offering as a way of saying thank you for being a part of the community.

 
LVL 62

Assisted Solution

by:Julian Hansen
Julian Hansen earned 332 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 332 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 83

Expert Comment

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

Accepted Solution

by:
leakim971 earned 1004 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

Free Tool: Site Down Detector

Helpful to verify reports of your own downtime, or to double check a downed website you are trying to access.

One of a set of tools we are providing to everyone as a way of saying thank you for being a part of the community.

Question has a verified solution.

Are you are experiencing a similar issue? Get a personalized answer when you ask a related question.

Have a better answer? Share it in a comment.

Join & Write a Comment

Introduction Chart.js, used properly, can visually add a difference to your charting applications. It engages your visitors and allows them to interact with data they otherwise wouldn't be able to without expensive and complicated systems. For this…
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 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)
The viewer will learn the basics of jQuery including how to code hide show and toggles. 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…

588 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