Handling data from an HTML table using jQuery

Question

I have the following table:

<table>
    <tr class="rowUpdate">
    <td>Corredor</td>
    <td>Id Corrdor
        <input type="text" value="dfdfgdf23231fg" class="validate" name="idcorreo" />
    </td>
    <td>Nombre
        <input type="text" value="rertretert" class="validate" name="nombre" />
    </td>
    <td>Email
        <input type="text" value="[email protected]" class="validate" name="email" />
    </td>
    <td>Empressa
        <input type="text" value="dfdfdf" class="validate" name="Empressa" />
    </td>
    <td>Pagina Web
        <input type="text" value="dfdfdf" class="validate" name="paginaWeb" />
    </td>
    <td>Telefono
        <input type="text" value="34454355" class="validate" name="telephon" />
    </td>
    <td>Cellular
        <input type="text" value="2323" class="validate" name="cellular" />
    </td>
    <td>
        <input type="button" id="updateBtn" value="Update" name="Update" style="float:left" />
    </td>
</tr>
</table>

To handle this, I have the following jQuery code:

 $(document).on('click', '#updateBtn', function () {
    var locations = [],
        rows = [],
        content = {};
    $('.rowUpdate').each(function (i) {
        var feed = [];

        $(this).find('td').each(function (j, v) {
            var label = $(this).html();
            if (j == 0) feed = feed + label;
            if (j != 0) {

                var input = $("input", this),
                    name = input.attr("name").substring(0, input.attr("name").length),
                    value = input.val();
                //alert(value);
                content[name] = value;
                //alert(JSON.stringify(content));
            }
            if (j == 7) {
                feed = feed + '=' + JSON.stringify(content);
            }
        });
        alert(feed);
        rows.push(feed);
    });

});

Can there be a better script to handle this? What I am trying to do is to save the content of the row in a form of string in the array row.

Answer 1

$(document).on('click', '#updateBtn', function () {    // See "Event Handler"
    var locations = [],                                // See "Unused Variable"
        rows = [],                                     // See "Improper Scope"
        content = {};

    $('.rowUpdate').each(function (i) {                // See "Exactly One" and "Parameters"
        var feed = [];

        $(this).find('td').each(function (j, v) {      // See "Parameters"
            var label = $(this).html();

            if (j == 0) feed = feed + label;           // See "Type Variance"
            if (j != 0) {
                var input = $("input", this),
                                                       // See "Wait, what?"
                    name = input.attr("name").substring(0, input.attr("name").length),
                    value = input.val();

                //alert(value);                        // See "Commented Code" and "Alert"
                content[name] = value;
                //alert(JSON.stringify(content));
            }
            if (j == 7) {
                                                       // See also "JSON"
                feed = feed + '=' + JSON.stringify(content);
            }
        });
        alert(feed);
        rows.push(feed);                               // See "Improper Scope"
    });
});

Event Handler - You've attached the event listener to the document which is fine, but you've used an Id selector to filter it to. There can be only one element with in the document referenced by that Id.¹ Unless the #updateBtn element is expected to be destroyed and recreated during the documents lifetime, it would be better to specify that element explicitly.

Unused Variable - The locations variable is never referenced after it is instantiated, remove it.

Improper Scope - The rows variable is pushed to, but never read from. If it is to be useful to other code it will need to be moved out of the event handler's scope.

The content variable is never used at this level of the scope, move its declaration to the narrowest scope required.

Exactly One - According to the HTML snippet you've provided, the .rowUpdate contains the #updateBtn which forces me to assume means there will be only 1 .rowUpdate. Update the markup to indicate this element is special by setting the id instead of the class attribute.

Because we expect this .each function to be invoked once it can be flattened into the parent scope.

Parameters - JavaScript does not allow functions to be overloaded by parameters.² the i and v parameters are never used and can be removed. Parameters are essentially variable declarations and should be named accordingly. j would be better named index.

Type Variance - feed is originally instantiated as an empty array, but you're now adding a string to it. I'm not sure what your intent is, but the effect will be to cast the empty array to an empty string and append label. This is only being done for the 0th element so a simple assignment will do.

Wait, what? - First, nearly every string literal in your code is enclosed in single quotes ('). Suddenly you've begun using double quotes ("). Javascript will allow either, but keep your code consistent.

I'm not sure what the purpose of the .substring method call when your arguments are the full length of the string. this can be removed.

Commented Code - Remove code that is unused. Only comment out code for the purposes of debugging or experimenting and remove the comment (and code if necessary) when done.

Alert - alert should not be used for debugging purposes. Use the console methods trace, debug, log, warn, etc..

JSON - JSON is a format used for serializing data. It should be used to save, restore, and transfer data. Your code should never manipulate JSON directly. Instead use the objects serialized from the JSON string.

---

After modifying the code based on the review, I end up with something like this:

var rows = [];

$('#updateBtn').on('click', function {
    var feed = {},
        content = {},
        label = $('.rowUpdate td').first().html();

    $('tr.rowUpdate td input[type=text]', this).each(function(index, input) {
        var $input = $(input),
            name = $input.attr('name'),
            value = $input.val();

        console.debug('value', value);
        content[name] = value;
        console.debug('content', content);
    });

    feed[label] = content;
    console.debug('feed', feed);
    rows.push(feed);
});

Then use the actual objects from the rows array if being consumed in other javascript code or send the rows JSON to wherever it is consumed. If for some reason your consumer cannot be modified to use a proper object and relies on the broken JSON string originally in the method, change feed[label] = content to feed = label + '=' + JSON.stringify(content).