How to simplify or dry syntax of this jquery code

Question

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?

user113716 · Accepted Answer · 2011-02-23 17:45:08Z

5

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"

answered Feb 23, 2011 at 17:45

user113716

323k64 gold badges454 silver badges441 bronze badges

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

Comments

Ken Redler · Accepted Answer · 2011-02-23 17:46:11Z

2

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...

answered Feb 23, 2011 at 17:46

Ken Redler

23.9k8 gold badges60 silver badges69 bronze badges

Comments

hvgotcodes · Accepted Answer · 2011-02-23 17:44:24Z

1

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');

answered Feb 23, 2011 at 17:44

hvgotcodes

121k33 gold badges209 silver badges237 bronze badges

Comments

macarthy · Accepted Answer · 2011-02-23 17:45:37Z

1

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');

answered Feb 23, 2011 at 17:45

macarthy

3,0692 gold badges24 silver badges24 bronze badges

Comments

Dan Inactive · Accepted Answer · 2011-02-23 17:47:36Z

1

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

answered Feb 23, 2011 at 17:47

Dan Inactive

10.1k19 gold badges52 silver badges71 bronze badges

Comments

T.J. Crowder · Accepted Answer · 2011-02-23 17:48:23Z

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.

Collectives™ on Stack Overflow

How to simplify or dry syntax of this jquery code

6 Answers 6

Comments

Comments

Comments

Comments

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

6 Answers 6

Comments

Comments

Comments

Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related