2
\$\begingroup\$

How can I optimize this code?

for(var i = 0; i<people_to_save_parse.length;i++)
{
 if($(".ID-"+people[i].tdid).is(':checked')==true)
 {
 people_to_save_parse[i].id = $(".ID-text-"+people[i].tdid).text();
 }else{
 people_to_save_parse[i].id = "";
 }
 if($(".Name-"+people[i].tdid).is(':checked')==true)
 {
 people_to_save_parse[i].firstname = $(".Name-text-"+people[i].tdid).text();
 }else{
 people_to_save_parse[i].firstname = "";
 }
 if($(".Surname-"+people[i].tdid).is(':checked')==true)
 {
 people_to_save_parse[i].lastname = $(".Surname-text-"+people[i].tdid).text();
 }else{
 people_to_save_parse[i].lastname = "";
 }
}
Jamal
35.2k13 gold badges134 silver badges238 bronze badges
asked Jul 24, 2014 at 2:52
\$\endgroup\$
3
  • 1
    \$\begingroup\$ What does this code do? \$\endgroup\$ Commented Jul 24, 2014 at 2:53
  • \$\begingroup\$ Check if switch is checked and do with some data something \$\endgroup\$ Commented Jul 24, 2014 at 2:54
  • 5
    \$\begingroup\$ Welcome to Code Review. Your question just came up in the 'First Post Review Queue'. Your question would be significantly better if you could add some more information about what it does. Also, what do you mean by optimize? Make it go faster, or make it more readable, smaller, etc.? \$\endgroup\$ Commented Jul 24, 2014 at 3:25

1 Answer 1

6
\$\begingroup\$

With a function:

var checkedValue = function (tdid, field) {
 if ($("." + field + "-" + tdid).is(':checked')) {
 return $("." + field + "-text-" + tdid).text();
 }
 return "";
}

and you could then simply call that with:

for(var i = 0; i < people_to_save_parse.length; i++) {
 people_to_save_parse[i].id = checkedValue(people[i].tdid, "ID");
 people_to_save_parse[i].firstname = checkedValue(people[i].tdid, "Name");
 people_to_save_parse[i].lastname = checkedValue(people[i].tdid, "Surname");
}

This removes much of the code duplication.

Note also that convention puts the { at the end of the line in JavaScript, not at the start of the next line. Also, you should add space between your operators and expressions to make them more readable. Finally, you don't need to add the == true as it is not useful in the if condition since the is(":checked") function returns a boolean already.

A concern I have is that you have two arrays, people_to_save_parse and people. You index them both with the same i index, and there is no apparent control that ensures they are 'in sync'. Are they a 1-to-1 matching pair of arrays?

answered Jul 24, 2014 at 4:33
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.