2
\$\begingroup\$

I am trying to use jQuery to make calculations with a table containing football data. Can I improve what I have done here? Is my code well-structured and executed, or does it need improvements?

http://jsfiddle.net/userdude/kCanv/

HTML - Table

<table id="table-pts">
<thead>
    <tr>
        <th colspan="3">&nbsp;</th>
        <th colspan="5">HOME</th>
        <th colspan="5">AWAY</th>
        <th colspan="5">SUMMARY</th>
        <th colspan="2">&nbsp;</th>
    </tr>
    <tr>
        <th>Pos</th>
        <th class="team">Team</th>
        <th>GP</th>
        <th>W</th>
        <th>D</th>
        <th>L</th>
        <th>F</th>
        <th>A</th>
        <th>W</th>
        <th>D</th>
        <th>L</th>
        <th>F</th>
        <th>A</th>
        <th>W</th>
        <th>D</th>
        <th>L</th>
        <th>F</th>
        <th>A</th>
        <th>GD</th>
        <th>Pts</th>
    </tr>
</thead>
<tbody>
        <tr id="1" class="edit_tr">
            <td></td>
            <td class="team">
            &nbsp;
            </td>
            <td><span id="summary_GP_1"></span></td>
            <td>
                <span id="home_w_1" class="text">1</span>
                <input type="text" value="1" class="editbox" id="home_w_input_1"/>
            </td>
            <td>
                <span id="home_d_1" class="text">3</span>
                <input type="text" value="3" class="editbox" id="home_d_input_1"/>
            </td>
            <td>
                <span id="home_l_1" class="text">2</span>
                <input type="text" value="2" class="editbox" id="home_l_input_1"/>
            </td>
            <td>
                <span id="home_f_1" class="text">4</span>
                <input type="text" value="4" class="editbox" id="home_f_input_1"/>
            </td>
            <td>
                <span id="home_a_1" class="text">76</span>
                <input type="text" value="76" class="editbox" id="home_a_input_1"/>
            </td>
            <td>
                <span id="away_w_1" class="text">8</span>
                <input type="text" value="8" class="editbox" id="away_w_input_1"/>
            </td>
            <td>
                <span id="away_d_1" class="text">9</span>
                <input type="text" value="9" class="editbox" id="away_d_input_1"/>
            </td>
            <td>
                <span id="away_l_1" class="text">10</span>
                <input type="text" value="10" class="editbox" id="away_l_input_1"/>
            </td>
            <td>
                <span id="away_f_1" class="text">11</span>
                <input type="text" value="11" class="editbox" id="away_f_input_1"/>
            </td>
            <td>
                <span id="away_a_1" class="text">12</span>
                <input type="text" value="12" class="editbox" id="away_a_input_1"/>
            </td>
            <td><span id="summary_w_1" ></span></td>
            <td><span id="summary_d_1" class="text"></span></td>
            <td><span id="summary_l_1" class="text"></span></td>
            <td><span id="summary_f_1" class="text"></span></td>
            <td><span id="summary_a_1" class="text"></span></td>
            <td><span id="summary_GD_1" class="text"></span></td>
            <td><span id="summary_Pts_1" class="text"></span></td>
        </tr>
        <tr id="2" class="edit_tr">
            <td></td>
            <td class="team">
            &nbsp;
            </td>
            <td><span id="summary_GP_2"></span></td>
            <td>
                <span id="home_w_2" class="text">1</span>
                <input type="text" value="1" class="editbox" id="home_w_input_2"/>
            </td>
            <td>
                <span id="home_d_2" class="text">3</span>
                <input type="text" value="3" class="editbox" id="home_d_input_2"/>
            </td>
            <td>
                <span id="home_l_2" class="text">2</span>
                <input type="text" value="2" class="editbox" id="home_l_input_2"/>
            </td>
            <td>
                <span id="home_f_2" class="text">4</span>
                <input type="text" value="4" class="editbox" id="home_f_input_2"/>
            </td>
            <td>
                <span id="home_a_2" class="text">76</span>
                <input type="text" value="76" class="editbox" id="home_a_input_2"/>
            </td>
            <td>
                <span id="away_w_2" class="text">8</span>
                <input type="text" value="8" class="editbox" id="away_w_input_2"/>
            </td>
            <td>
                <span id="away_d_2" class="text">9</span>
                <input type="text" value="9" class="editbox" id="away_d_input_2"/>
            </td>
            <td>
                <span id="away_l_2" class="text">10</span>
                <input type="text" value="10" class="editbox" id="away_l_input_2"/>
            </td>
            <td>
                <span id="away_f_2" class="text">11</span>
                <input type="text" value="11" class="editbox" id="away_f_input_2"/>
            </td>
            <td>
                <span id="away_a_2" class="text">12</span>
                <input type="text" value="12" class="editbox" id="away_a_input_2"/>
            </td>
            <td><span id="summary_w_2" ></span></td>
            <td><span id="summary_d_2" class="text"></span></td>
            <td><span id="summary_l_2" class="text"></span></td>
            <td><span id="summary_f_2" class="text"></span></td>
            <td><span id="summary_a_2" class="text"></span></td>
            <td><span id="summary_GD_2" class="text"></span></td>
            <td><span id="summary_Pts_2" class="text"></span></td>
        </tr>    
    </tbody>
</table>

CSS

#table-pts {
    color: #333;
    text-align: center;
    font-size: 12px;
    width: 100%;
}
#table-pts tbody tr {
    border-bottom: 1px solid #dddcdc;
    padding: 10px;
    line-height: 30px;
}
#table-pts tbody td{
    vertical-align: middle;
    padding: 5px 0;
}

#table-pts thead tr th{
    text-align: center;
    line-height: 30px;
    background: #233825;
    color: #FFF;
    border-left: 1px solid #CCC;

    width: 30px;
}
#table-pts .table-team{
    width: 10px;
    text-align: left;
    padding: 3px 7px 3px 7px;
}
#table-pts .editbox{
    display: none;
    border: 1px solid #CCC;
    text-align: center;
}

#table-pts .odd{background: #fafafa;}
.table-hl
{
    width: 70px;
}
#table-pts input{
    width: 20px;
}

jQuery

$(document).ready(function(){
    $(".edit_tr").click(function(){
        var ID = $(this).attr('id');

        $("#home_w_"+ID).hide();
        $("#home_d_"+ID).hide();
        $("#home_l_"+ID).hide();
        $("#home_f_"+ID).hide();
        $("#home_a_"+ID).hide();
        $("#away_w_"+ID).hide();
        $("#away_d_"+ID).hide();
        $("#away_l_"+ID).hide();
        $("#away_f_"+ID).hide();
        $("#away_a_"+ID).hide();

        $("#home_w_input_"+ID).show();
        $("#home_d_input_"+ID).show();
        $("#home_l_input_"+ID).show();
        $("#home_f_input_"+ID).show();
        $("#home_a_input_"+ID).show();
        $("#away_w_input_"+ID).show();
        $("#away_d_input_"+ID).show();
        $("#away_l_input_"+ID).show();
        $("#away_f_input_"+ID).show();
        $("#away_a_input_"+ID).show();
    }).change(function(){
        // calculate point in table football
        var ID = $(this).attr('id');

        var home_w = parseInt($("#home_w_input_"+ID).val());
        var home_d = parseInt($("#home_d_input_"+ID).val());
        var home_l = parseInt($("#home_l_input_"+ID).val());
        var home_f = parseInt($("#home_f_input_"+ID).val());
        var home_a = parseInt($("#home_a_input_"+ID).val());
        var away_w = parseInt($("#away_w_input_"+ID).val());
        var away_d = parseInt($("#away_d_input_"+ID).val());
        var away_l = parseInt($("#away_l_input_"+ID).val());
        var away_f = parseInt($("#away_f_input_"+ID).val());
        var away_a = parseInt($("#away_a_input_"+ID).val());

        var summaryW = home_w + away_w;
        var summaryD = home_d + away_d;
        var summaryL = home_l + away_l;
        var summaryF = home_f + away_f;
        var summaryA = home_a + away_a;
        var summaryGD = summaryF + summaryA;
        var summaryPts = summaryW * 3 +summaryD * 1;
        var summaryGP = summaryW + summaryD + summaryL;

        /* var dataString = 'id='+ ID + '&home_w='+ home_w + '&home_d='+ home_d + '&home_l='+ home_l + '&home_f='+ home_f + '&home_a='+ home_a + '&away_w='+ away_w + '&away_d='+ away_d + '&away_l='+ away_l + '&away_f='+ away_f + '&away_a='+ away_a + '&action=edit_pts';
        $("#home_w_"+ID).html('<img src="load.gif"/>');

        $.ajax({
        type: "POST",
        url: "table_pts_action_ajax.php",
        data: dataString,
        cache: false,
        success: function(html)
        {*/

        // live update in my table football
        $("#home_w_"+ID).html(home_w);
        $("#home_d_"+ID).html(home_d);
        $("#home_l_"+ID).html(home_l);   
        $("#home_f_"+ID).html(home_f);
        $("#home_a_"+ID).html(home_a);
        $("#away_w_"+ID).html(away_w);
        $("#away_d_"+ID).html(away_d);
        $("#away_l_"+ID).html(away_l);
        $("#away_f_"+ID).html(away_f);
        $("#away_a_"+ID).html(away_a);

        $("#summary_w_"+ID).html(summaryW);
        $("#summary_d_"+ID).html(summaryD);
        $("#summary_l_"+ID).html(summaryL);
        $("#summary_f_"+ID).html(summaryF);
        $("#summary_a_"+ID).html(summaryA);
        $("#summary_GD_"+ID).html(summaryGD);
        $("#summary_Pts_"+ID).html(summaryPts);
        $("#summary_GP_"+ID).html(summaryGP);
    });

    $("#table-pts tbody tr").each(function(){
        // page is load finish.. calculate point in table football
        var ID = $(this).attr('id');

        var txtHomeW = parseInt($("#home_w_"+ID).text());
        var txtHomeD = parseInt($("#home_d_"+ID).text());
        var txtHomeL = parseInt($("#home_l_"+ID).text());
        var txtHomeF = parseInt($("#home_f_"+ID).text());
        var txtHomeA = parseInt($("#home_a_"+ID).text());
        var txtAwayW = parseInt($("#away_w_"+ID).text());
        var txtAwayD = parseInt($("#away_d_"+ID).text());
        var txtAwayL = parseInt($("#away_l_"+ID).text());
        var txtAwayF = parseInt($("#away_f_"+ID).text());
        var txtAwayA = parseInt($("#away_a_"+ID).text());

        var summaryW = parseInt(txtHomeW+txtAwayW);
        var summaryD = parseInt(txtHomeD+txtAwayD);
        var summaryL = parseInt(txtHomeL+txtAwayL);
        var summaryF = parseInt(txtHomeF+txtAwayF);
        var summaryA = parseInt(txtHomeA+txtAwayA);
        var summaryGD = parseInt(summaryF-+summaryA);
        var summaryPts = parseInt(summaryW * 3 + summaryD * 1);
        var summaryGP = parseInt(summaryW + summaryD + summaryL);

        $(this).find("#summary_w_"+ID).text(summaryW);
        $(this).find("#summary_d_"+ID).text(summaryD);
        $(this).find("#summary_l_"+ID).text(summaryL);
        $(this).find("#summary_f_"+ID).text(summaryF);
        $(this).find("#summary_a_"+ID).text(summaryA);
        $(this).find("#summary_GD_"+ID).text(summaryGD);
        $(this).find("#summary_Pts_"+ID).text(summaryPts);
        $(this).find("#summary_GP_"+ID).text(summaryGP);
    });

    // Edit input box click action
    $(".editbox").mouseup(function(){
        return false;
    });

    // Outside click action
    $(document).mouseup(function(){
        $(".editbox").hide();
        $(".text").show();
    });
});
\$\endgroup\$
  • 3
    \$\begingroup\$ You need to use classes where you've got all of those ids being repetitively called. If you use a class, you can use DOM traversal techniques like $.siblings('.summary_w_d') instead of looping exhaustively over each row like that. When you see that kind of thing, it really should be refactored. \$\endgroup\$ – Jared Farrish Jun 18 '12 at 6:07
5
\$\begingroup\$

If you see lines that look awfully similar in your code, usually it's a strong smell and cry for some refactoring. Much of your code could be generalize to be handled by a few helper functions.

Also, using some arrays (or other data structures) to list the elements to watch and modify, you could easily iterate over them and pass them to these helper functions.

\$\endgroup\$
0
\$\begingroup\$

I second @haylem's suggestions, and add:

Invalid ids

The ids of your tr are invalid. They must begin with a letter. Perhaps line1 instead of 1?
Then your other ids could be classes, and you could target them as #line2 .home_a instead of #home_a_1.

Register handlers with .on()

You should also use JQuery 1.7's .on() syntax:

$('#table-pts .edit_tr').on({
    'click': function () {
        //...
    },
    'change': function() {
        //...
    }
});

Use + to coerce numbers, not parseInt(a)

Use the unary operator + to coerce numeric values. It is not only easier, but also safer than parseInt(): https://stackoverflow.com/a/2243631/148412

So,
var home_w = +$("#home_w_input_"+ID).val();
instead of
var home_w = parseInt($("#home_w_input_"+ID).val());

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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