Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I have this fiddle http://jsfiddle.net/L7zuT/ where I'm working on a small part of a big event program. The functionality is ok, the part of the problem would be the large JavaScript lines needed with the full event program. Is there any way to implement it with DRY (don't repeat yourself) or a better approach for a simpler way?

<table cellspacing="0" cellpadding="0" border="1">

  <tr>
    <td>13:00-14:30</td>
    <td rowspan="2"><label for="date_main01">18.AAL Solution Deployment Seminar</label></td>
      <td rowspan="2"><label for="date_main02">9. Results of the AALIANCE Network: Roadmap and more</label></td>
      <td><label for="date_sub03">7. From    actimetry to ADL ,in the framework of remote medical monitoring of elderly    from living labs to the nursing home</label></td>
      <td rowspan="2"><label for="date_main05">8. Interoperability APIs and LivingLabs for AAL</label></td>
      <td rowspan="2"><label for="date_main06">10. Large scale deployment of AAL solutions based on Open    Platform: Challenge and perspectives</label></td>
    <td>&nbsp;</td>
  </tr>
  <tr>
    <td>14:30-16:00</td>
      <td><label for="date_sub04">1.    User Forum and workshop for exergames in AAL</label></td>
    <td>&nbsp;</td>
  </tr>
    <tr>
        <td>16:00-17.30</td>
        <td><label for="date_last07">19.Deployment event - matchmaking session</label></td>
        <td colspan="5">&nbsp;</td>
    </tr>
</table>
<input id="date_main01" name="date_main01" type="radio" value="01" />
<input id="date_main02" name="date_main01" type="radio" value="02" />
<input id="date_sub03" name="date_sub01" type="radio" value="03" />
<input id="date_sub04" name="date_sub02" type="radio" value="04" />
<input id="date_main05" name="date_main01" type="radio" value="05" />
<input id="date_main06" name="date_main01" type="radio" value="06" />
<input id="date_last07" name="date_last01" type="radio" value="07" />

CSS

tr {
    height: 1px;
}
td{
    height: 100%;
}
label{
    display: block; 
    min-height: 100%; /* for the latest browsers which support min-height */
    height: auto !important; /* for newer IE versions */
    height: 100%; /* the only height-related attribute that IE6 does not ignore  */
}
label:hover {
    cursor:pointer;
}

JS

$(document).ready(function(){
    $("input[name='date_main01']").click(function() {
        var test = $(this).val();
        $("input[name='date_sub01']").prop('checked', false);
        $("input[name='date_sub02']").prop('checked', false);
        $("label[for*='date_main']").css("background", "none");
        $("label[for*='date_sub']").css("background", "none");
        $("label[for='date_main"+test+"']").css("background-color", "yellow");            
    });
    $("input[name='date_sub01']").click(function() {
        var test = $(this).val();
        $("input[name$='date_main01']").prop('checked', false);
        $("label[for*='date_main']").css("background", "none");
        $("label[for='date_sub"+test+"']").css("background-color", "yellow");            
    });
    $("input[name='date_sub02']").click(function() {
        var test = $(this).val();
        $("input[name$='date_main01']").prop('checked', false);
        $("label[for*='date_main']").css("background", "none");
        $("label[for='date_sub"+test+"']").css("background-color", "yellow");            
    });
        $("input[name='date_last01']").click(function() {
        var test = $(this).val();
        $("label[for='date_last"+test+"']").css("background-color", "yellow");            
    }); 
});
share|improve this question
add comment

3 Answers

 $("input[name='date_sub01']").prop('checked', false);
 $("input[name='date_sub02']").prop('checked', false);

Can be turned into:

$("input[name='date_sub01'], input[name='date_sub02']").prop('checked', false);

Also if you assign the selectors to a variable your code will be much faster:

var sub_01 = $("input[name='date_sub01']")

You never need to use the method .val().

If you do:

var test = this.value;

You will get the same results without accessing jQuery.

Since you are using background:none alot, you can keep it in an object:

noneBackground: {"background":"none"}

and pass it as an argument

  $("label[for*='date_main']").css(noneBackground);
share|improve this answer
add comment

I think selectors which are used at multiple places can be move at top and keep them in variable will optimize the code at certain level and increase speed.

var ipDM01 = $("input[name$='date_main01']");
share|improve this answer
    
Welcome to Code Review! You say this increases the speed of the code, but do you know by how much? Is it a negligible difference (as it would seem)? –  Alex L Jun 25 at 19:50
add comment

Combine the following click event handlers

$("input[name='date_sub01']").click(function() {
    var test = $(this).val();
    $("input[name$='date_main01']").prop('checked', false);
    $("label[for*='date_main']").css("background", "none");
    $("label[for='date_sub"+test+"']").css("background-color", "yellow");            
});
$("input[name='date_sub02']").click(function() {
    var test = $(this).val();
    $("input[name$='date_main01']").prop('checked', false);
    $("label[for*='date_main']").css("background", "none");
    $("label[for='date_sub"+test+"']").css("background-color", "yellow");            
});

Into one,

$("input[name='date_sub02'],input[name='date_sub01']").click(function() {
    var test = this.value;
    $("input[name$='date_main01']").prop('checked', false);
    $("label[for*='date_main']").css("background", "none");
    $("label[for='date_sub"+test+"']").css("background-color", "yellow");            
});

Suggestion: Instead of adding inline styles using .css(), you can always use a CSS class, it is simple to remove/add properties with this way.

share|improve this answer
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.