Solved

Better way of using callback in javascript

Posted on 2016-11-01
7
54 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 42

Expert Comment

by:Rob Jurd, EE MVE
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 your wiki can always stay up-to-date

Quip doubles as a “living” wiki and a project management tool that evolves with your organization. As you finish projects in Quip, the work remains, easily accessible to all team members, new and old.
- Increase transparency
- Onboard new hires faster
- Access from mobile/offline

 
LVL 51

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

What Security Threats Are You Missing?

Enhance your security with threat intelligence from the web. Get trending threat insights on hackers, exploits, and suspicious IP addresses delivered to your inbox with our free Cyber Daily.

Join & Write a Comment

Introduction Knockoutjs (Knockout) is a JavaScript framework (Model View ViewModel or MVVM framework).   The main ideology behind Knockout is to control from JavaScript how a page looks whilst creating an engaging user experience in the least …
Nothing in an HTTP request can be trusted, including HTTP headers and form data.  A form token is a tool that can be used to guard against request forgeries (CSRF).  This article shows an improved approach to form tokens, making it more difficult to…
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)

744 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

Need Help in Real-Time?

Connect with top rated Experts

10 Experts available now in Live!

Get 1:1 Help Now