CODEWITHSUNDEEP
javascriptjqueryrefactoring
twitter
twitter

Reputation: 9295

How to simplify or dry syntax of this jquery code

I'm doing the same selection on a whole bunch of radio button groups. The only thing that changes is the name. 

var fcolor = $(this).closest('.branch').find('input[name="fcolor"]:checked').val();
var bcolor = $(this).closest('.branch').find('input[name="bcolor"]:checked').val();
var sidec = $(this).closest('.branch').find('input[name="sidec"]:checked').val();
var linec = $(this).closest('.branch').find('input[name="linec"]:checked').val();

How do I simplify this code so I'm not repeating code like this?

Upvotes: 2

Views: 239

Answers (6)

T.J. Crowder
T.J. Crowder

Reputation: 1074138

The first thing is to recognize that when you call $(this), jQuery does at least a couple of function calls and a memory allocation. So you really want to cache the result and reuse it.

The second thing is that find has to do work, so, again, cache the result:

var branch = $(this).closest('.branch');
var fcolor = branch.find('input[name="fcolor"]:checked').val();
var bcolor = branch.find('input[name="bcolor"]:checked').val();
var sidec = branch.find('input[name="sidec"]:checked').val();
var linec = branch.find('input[name="linec"]:checked').val();

No, there's still some repeating there; you could create a function for "get me the value of the checkbox matching X":

function getCheckedValue(ancestor, name) {
    return ancestor.find('input[name=' + name + ']:checked').val();
}

So then:

var branch = $(this).closest('.branch');
var fcolor = getCheckedValue(branch, 'fcolor');
var bcolor = getCheckedValue(branch, 'bcolor');
var sidec  = getCheckedValue(branch, 'sidec');
var linec  = getCheckedValue(branch, 'linec');

And then, if you really want, you can get into having a list of these names and looping through it, at which point depending on your situation, it may be perfectly justifiable, or it may be complexity you don't need.

Upvotes: 1

Dan Inactive
Dan Inactive

Reputation: 10060

Here's an idea:

var closest = $(this).closest('.branch'); // don't re-compute this every time
var inputs = ['fcolor','bcolor','sidec','linec']; // these are the names you'll be looking for, add as many as you need
var values = {}; // here's where you will store your values
for (var i in inputs) {
    values[inputs[i]] = closest.find('input[name="' + inputs[i] + '"]:checked').val();
}

You would then read your values as: values.fcolor

Upvotes: 1

Ken Redler
Ken Redler

Reputation: 23943

Assuming you have a specific list of names (and not all names), you could try something like this:

var names = [
  'fcolor',
  'bcolor',
  'sidec',
  'linec'
];

var $inputs = $(this).closest('.branch').find('input:checked');
var values = {};

$.each( names, function(i,v){
  values[v] = $inputs.filter('[name='+v+']').val();
});

Now you have:

values.fcolor
values.bcolor
etc...

Upvotes: 2

macarthy
macarthy

Reputation: 3069

var linec =   getval($(this).closest('.branch'),'linec');


function getval(parent,name) {
return parent.find('input:[' + name + ']:checked').val();
}

EDIT:

This is quicker too...

var branch = $(this).closest('.branch');

    var linec =   getval(branch,'linec');

Upvotes: 1

user113716
user113716

Reputation: 322462

If you're interested in all inputs with a name attribute, I'd select them all, the create an object of properties and values.

If you need to single out certain ones, give them a common class, and select them by that.

var props = {};

$(this).closest('.branch').find('input[name]:checked').each(function() {
    props[ this.name ] = this.value;
});

You'll end up with a structure like this:

props = {
    fcolor: "some value",
    bcolor: "some value",
    sidec: "some value",
    linec: "some value"
};

...accessible as:

props.fcolor;  // "some value"

Upvotes: 5

hvgotcodes
hvgotcodes

Reputation: 120178

create a function. from what i see, everything is the same except the input name. pass the name in and construct your string....

something like

function myOperation(name) {
    return $(this).closest('.branch').find('input[name="' + name + '"]:checked').val();
}

you could also pass in the scope like 

function myOperation(scope, name) {
    return $(scope).closest('.branch').find('input[name="' + name + '"]:checked').val();
}

and then execute like

   myOperation(this, 'fcolor');

Upvotes: 1

Related Questions