Addition of accession analysis stubs to new requests and accessions

There's things I know about JavaScript and jQuery, and things I don't. I know that this works, gets and stores what I want so that, when I need to do stuff, I don't have to wait for it. Functionally, I like this. But I'm the only JavaScript guy in my shop, and so I don't see JavaScript I didn't write all that often.

I've heard recently that it is considered wrong to fill up the name space, so you use objects to hold everything. I think I'm doing this right now, but I'm not sure.

And in adding stuff to the DOM, I believe I'm a bit more verbose than I need to be, but I don't really know what to change.

Any comments?

```
// This uses Javascript, jQuery and jQuery-UI to allow for the addition of accession analysis stubs
// to new requests and accessions

var aa_stub = {

// variables
ajax : 'MY URL HERE' ,

// data storage
data : {} ,
references : {} ,
reference_order : [] ,
analysis : {} ,
analysis_order : [] ,

// functions

// ---- ---- ---- ---- ---- ---- ---- ---- ---- ---- ---- ---- ---- ---- ---- ----
// dialog popped up when "Add AA Stub' button is pressed
do_dialog : function ( sample_num ) {
$( '' )
.attr( 'id' , 'add_new_stub' )
.attr( 'title' , 'Add Stub' )
.dialog()
$('.ui-icon-closethick').text( 'X' )
$('.ui-dialog-titlebar-close')
.click( function () {
$( '#add_new_stub' ).remove()
} )
$( '' )
.appendTo( '#add_new_stub' )
.attr( 'id' , 'instructions' )
.text( 'Enter a reference, an analysis, any extra parameters, then click Go' )

$( '' )
.appendTo( '#add_new_stub' )
.attr( 'id' , 'ref_box' )
$( '' )
.appendTo( '#ref_box' )
.text( 'Reference Species' )
$( '' )
.appendTo( '#ref_box' )
.attr('id' ,'reference')
for ( i in aa_stub.reference_order

Let me start with few common observations.

It's better to generate a complete DOM element before you attach it to DOM tree (add to page) if you can, because it tends to be faster. For example setting its text after can trigger another reflow. Do it often enough and it start to matter. Not every change is expensive (e.g. setting ID before or later doesn't matter much), but I think it also reads better (step 1.: build node, step 2: attach it).

If you are building a more complex HTML (DOM subtree), it's also better to create a document fragment, add nodes to it and then add that fragment to page to avoid costly reflows.

It's good practice to declare all your variables at the top of function. They will exist from that point onward even if you declare them at the very bottom. Not doing this opens you to subtle errors.

Not every variable has to have its own var statement infront. E.g. it's OK to do var a = 5, b = 3 to declare a and b;

So on to concrete examples:

$( '' )
    .attr( 'id' , 'add_new_stub' )
    .attr( 'title' , 'Add Stub' )
    .dialog()

You could do this same thing in two ways:

$('').dialog();

or

$('', { id: 'add_new_stub', title: 'Add Stub })
    .dialog()

Personally I'd pick first version when dealing with elements that have fixed attribute values, since it's most obvious and second when attribute values are variables. Learn more.

Instead of for() loop over array, you can use

$.each(array, function (i, el) {
    ... your code ...
});

This way you can also localize variables better.

I don't understand the purpose of this part:

var number = 1
    $( bucket ).children( 'span' ).each( function () {
        var num = parseInt( $( this ).attr( 'number' ) )
        if ( number ' )
        .addClass( 'stub' )
        .attr( 'number' , number )

Correct me if I'm wrong, but you are trying to set number to 1 higher than highest, right? If yes, than that's an awfully expensive way to do this. You could improve this in two ways. Either you store current maximum or since new span with highest number is always added to the end of a list, just pick that one ("span:last") and read what maximum currently is.

You don't use aa_stub.data.flag anywhere.

Btw, I'm not fond of dropped colons at the end of lines and closing } at same depth as end of block, but that's a personal choice.

Anyway, that's a quick overlook of your code. I hope it helped.

StackExchange Code Review Q#4388, answer score: 6