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 = "";
}
}
-
1\$\begingroup\$ What does this code do? \$\endgroup\$Jamal– Jamal2014年07月24日 02:53:43 +00:00Commented Jul 24, 2014 at 2:53
-
\$\begingroup\$ Check if switch is checked and do with some data something \$\endgroup\$Arthur Yakovlev– Arthur Yakovlev2014年07月24日 02:54:46 +00:00Commented 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\$rolfl– rolfl2014年07月24日 03:25:08 +00:00Commented Jul 24, 2014 at 3:25
1 Answer 1
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?