Pulling JSON using jQuery and manipulating the Front-end

My JavaScript is fairly new and want to learn new ways to create clean, fast code. Can you have a look at this code and guide me on how to optimise this code to improve my skills? I have provided the HTML even though I am not able to edit the actual html as its generated but lets pretend I could..

What I am doing is pulling data from a Product JSON file and manipulating the data to be shown in the website.

function Details(el){
    var sku = $(el).attr("data");
    $.getJSON("/micro/"+sku, function(json) {
        if (json.price!=null) {
          $(el+' .p_img img').attr({src:json.images[2].url, alt:json.images[2].altText});
          $(el+' .p_img_lrg').attr("src",json.images[1].url);
          $(el+' .p_name').text(json.name);
          $(el+' .p_url').attr("href",json.url);
          $(el+' .p_p').text("£"+Number(json.p.value).toFixed(2));
          $(el+' .p_pu').text(json.pu);
          $(el+' .p_b_name').text(json.categories[0].name);
          $(el+' .p_b_url').attr("href",json.categories[0].url);
        if (json.oldPrice != null) {
          var pWas = ' .p_was',
              pSave = ' .p_save';
          $(el+pWas).text("was £"+json.old.value);
          $(el+pSave).text("save £"+(json.old.value-json.value));
          $(el+pSave).hide();
          $(el+pWas).css('margin-left',0).css('padding-left',0).addClass('clr').removeClass('left').css('float','none');
        }else{
          $(el+pSave,el+pWas).hide(); 
        }
    }

    if (json.stockLevel==0) { 
      $(el+' form.add').attr({'action':'/notification','method':'post'}).html('');
    } else { 
      $(el+' form.add').attr({'action':'/add','method':'post'}).html('');
    }
});
}

HTML:

    
        #
        test
    
    
        
    
    
        
        
            
        
            
                
                    now 
                    £
                
            
         
     
     View All  Products
  

Functions in JavaScript, by convention, begin with a lowercase letter, unless they're constuctors. So Details() should be details()

You've got some missing indentation, which makes the code harder to read. In general your code could probably benefit from a bit more whitespace to help legibility.

You also have some funky-looking strings, like: '' and works just fine with out any backslashes.

You can also use jQuery to build those elements for you, i.e.

form.append($("").attr({
  type:  hidden,
  name:  productCodePost,
  value: sku
});

which gives you a more structured and readable source code, instead of giant strings.

There's also a lot of repetition - or semi-repetition in this case: The el+"...". I'd advice something like what Matt suggested which is to store the result of $(el) once, and from there using .find() to locate the individual elements you want to update.

For the actual element updating there isn't that much that can be improved, since there's no discernible commonalities between, say, element IDs and JSON keys (in fact, the JSON seems very messy!)

There are some minor things you could do like define a renderAmount function to make sure you're always printing prices the same way:

function renderAmount(amount) {
  return "£" + Number(amount).toFixed(2);
}

Right now, you only use the .toFixed() in one place, although you have 3 different places where you need to write amounts.

Similarly, you could add a function to update images, since you do it at least twice. Not a great savings, but it helps break up the code.

function updateImage(element, info) {
  element.attr({
    src: json.url,
    alt: json.altText
  });
}

// ...

updateImage(container.find(".p_img img"), json.images[2]);

Lastly, what's going on the element that shows the savings? If there's no old price, it's just hidden, which makes sense. But if there is an old price, you update the .p_save element... and then hide it anyway?

StackExchange Code Review Q#44739, answer score: 5