Still celebrating National IT Professionals Day with 3 months of free Premium Membership. Use Code ITDAY17

x
?
Solved

Better way of using callback in javascript

Posted on 2016-11-01
7
Medium Priority
?
100 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
[X]
Welcome to Experts Exchange

Add your voice to the tech community where 5M+ people just like you are talking about what matters.

  • Help others & share knowledge
  • Earn cash & points
  • Learn & ask questions
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
Concerto Cloud for Software Providers & ISVs

Can Concerto Cloud Services help you focus on evolving your application offerings, while delivering the best cloud experience to your customers? From DevOps to revenue models and customer support, the answer is yes!

Learn how Concerto can help you.

 
LVL 59

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 82

Expert Comment

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

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

A new era in Cloud training has arrived.

A day that will go down in Cloud history.. But are you ready for it? Will you accept this Cloud challenge?

Question has a verified solution.

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

Boost your ability to deliver ambitious and competitive web apps by choosing the right JavaScript framework to best suit your project’s needs.
Today, the web development industry is booming, and many people consider it to be their vocation. The question you may be asking yourself is – how do I become a web developer?
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…
Suggested Courses

688 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