5

I am creating a commercial API for the first time for responsive webpages/web applications (mobile devices).

I am new and, sadly, working alone as well as new to Javascript (long complicated story).

I was just wondering if someone from the industry could offer their professional opinion on the following format of a "get" call:

var getSample = function(params) {
    //Returns Object
    return $.ajax({ 
        url: URL + 'downloadQuadrat.php',
        type: 'GET',
        data: { 'projectID': params.pid, 'quadratID': params.qid },
        dataType: dataType
    });
}

Function call:

var printList = function(lid,options,get) {
    var list = $("ul#"+lid);
    var promise = get(options);

    promise.promise().then(
        function(response) {
            var items = response;
            list.empty();

            $.each(items, function(item,details) {
                var ul = $('<ul/>');
                ul.attr('id', lid+'_'+details.ID);
                var li = $('<li/>')
                .text(details.ID)
                .appendTo(list);
                ul.appendTo(list);

                $.each(details,function(key,value) {
                    var li = $('<li/>')
                    .text(key+': '+value)
                    .appendTo(ul);
                });
            });
        }
    );
}

Any input or guidance will be hugely appreciated.

4
  • 9
    This kind of question may be better suited for http://codereview.stackexchange.com/ Commented Dec 12, 2012 at 0:48
  • Thanks, I will certainly check it out Commented Dec 12, 2012 at 0:51
  • 1
    Aside from the fact that codereview might be suitable, you do have real problems with your code. The getSample uses an asynchronous Ajax call, so you can’t return the value from the function. You will need to bring in a callback function as argument or return a promise object. Commented Dec 12, 2012 at 1:21
  • 1
    Code updated to include "promise" Commented Dec 12, 2012 at 2:21

2 Answers 2

3

I'm not a professional from the industry, per se, but there's a few things that I think would improve your code:

  • Format it according to common conventions. It's hard to see what your code is doing without proper indentation.
  • Just use $("#"+lid) instead of $("ul#"+lid). The ul at the beginning does not add any disambiguation because id attributes must be unique, and it just make it take longer to parse.
  • Ditch localstorage in this case. It's not supported on all browsers, and as far as I can tell, you don't need it here. Just directly use the data returned from the response.

Here is how I would change your code:

var printList = function(lid, options, get) {
    var promise = get(options);
    var list = $("#" + lid);

    promise.success(function(response) {
        var data = response;
        list.empty();
        $.each(data, function(item, details) {
            var ul = $('<ul/>').attr('id', lid + '_' + details.ID);
            var li = $('<li/>').text(details.ID).appendTo(list);
            ul.appendTo(list);
            $.each(details, function(key, value) {
                var li = $('<li/>').text(key + ': ' + value).appendTo(ul);
            });
        });
    });
}

EDIT: The edited version of your code looks fine to me, except for the minor ul# thing.

Sign up to request clarification or add additional context in comments.

2 Comments

Thank you. I will make the recommended changes. Is there any real difference between .promise().then() and .success() functionality-wise? Or will either do the trick?
There is a difference, but in this case I think they are functionally identical.
3

Some more suggestions to make your API a tad more professional looking:

1 - Namespacing

Use a namespace to keep your code packaged neatly in it's own space where it won't conflict with other function definitions on the page. Something like this to start with:

window.MyNamespace =  {};
MyNamespace.get = function(qid, pid) {
   //things
};
MyNamespace.anotherFunction = function() {
   //other stuff
}

If your code starts getting bigger you can wrap the whole lot in a closure. Also you could define it all as class and then instantiate it once to make your code neater and allow you to store instance variables and call this.anotherFunction(). I can provide examples of those too if you like.

2 - API method signatures

Another thing I prefer to see is explicit arguments to functions rather than function get(params) style code. Making parameters explicit makes your code easier to read and understand and discourages ad-hoc hacks which is especially important when writing an API. It's a case of just because you can doesn't mean you should.

3 - Config

Try to move things like IDs and URLs into variables to start with to make your code a bit easier to reuse and work with.

Generally, Javascript developers are famous for looking at your code before they look at your API docs so anything you can do to make the API function names and argument names more expressive and self-documenting will help them.

1 Comment

1 - I actually had namespaces prior to editing the OP. However, for the time being I have deprecated their use while I work out a solid structure to allow for ambiguity. 2 - As mentioned above, these particular get methods are called ambiguously. I had figured hard-typing the arguments will make it difficult to use delegates in UI calls like printList() 3 - You will see a "URL" variable in the $.ajax url property, that is a config var set outside of scope. I will mull over the idea of hard-typing arguments and try to find an elegant way to reintroduce namespaces :)

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.