CODEWITHSUNDEEP
javascriptjqueryperformance
ggwicz
ggwicz

Reputation: 568

Messy but Working jQuery

I've written the jQuery you'll see below for a little project I'm working on. It works perfectly and is all set, but, as you can see, it's messy and kind of...long winded.

I've tried a bunch of different ways to clean this up but I'm not just ninja-like enough to really neaten it up. Any advice? Thanks in advance guys! 

  var colspan = $(".col header span"),
      rowspan = $(".row header span"),
      topspan = $(".top header span");

  var colh2 = $(".col header h2").h2width();
  var rowh2 = $(".row header h2").h2width();
  var toph2 = $(".top header h2").h2width();

  var colwidth = 820 - colh2;
  var rowwidth = 820 - rowh2;
  var topwidth = 820 - toph2;

  colspan.css({float: 'left', width: colwidth});
  rowspan.css({float: 'left', width: rowwidth});
  topspan.css({float: 'left', width: topwidth});

Upvotes: 1

Views: 130

Answers (5)

Andrea
Andrea

Reputation: 724

Just use a function:

function updateStyle(name){
   var headerSpan = $('.' + name + ' header span');
   var headerH2 = $('.' + name + ' header h2');
   headerSpan.css({float: 'left', width: 820 - headerH2.h2width()});
}

updateStyle('col');
updateStyle('row');
updateStyle('top');

Upvotes: 0

ThiefMaster
ThiefMaster

Reputation: 318808

Simply get rid of the duplicate code:

$.each(['.col', '.row', '.top'], function(i, cls) {
    var width = $(cls + ' header h2').h2width();
    $(cls + ' header span').css({
        float: 'left',
        width: 820 - width
    });
});

Upvotes: 0

Tadeck
Tadeck

Reputation: 137460

I would probably rewrite your code in the following way:

var conts = {
    'col': jQuery('.col header'),
    'row': jQuery('.row header'),
    'top': jQuery('.top header')
};

jQuery.each(conts, function(index, val){
    val.find('span').css({
        'float': 'left',
        'width': 820-val.find('h2').h2width()
    });
});

This uses caching the main elements and then will iterate on all of them applying the similar actions.

See more information on jQuery's .each() function.

EDIT: Or even shorter:

jQuery('.col header, .row header, .top header').each(function(){
    var current = jQuery(this);
    current.find('span').css({
        'float': 'left',
        'width': 820 - current.find('h2').h2width()
    });
});

Upvotes: 0

Raynos
Raynos

Reputation: 169551

["col", "row", "top"].forEach(function (className) {
  var str = "." + className + " header";
  var h2s = document.querySelectorAll(str + " h2");
  var spans = document.querySelectorAll(str + " span");
  var width = 820 - h2width(h2s);
  Array.prototype.forEach.call(spans, function (span) {
    span.style.float = "left";
    span.style.width = width;
  });
});

Because jQuery is always overkill.

Upvotes: 2

Joseph Marikle
Joseph Marikle

Reputation: 78590

I would do it like this maybe? shorter but maybe not as well documented:

$(".col header span, .row header span, .top header span").each(function(){
    $(this).css({
        float: 'left',
        width: 820 - $(this).siblings("h2").width()
    });
});

Upvotes: 1

Related Questions