current community

your communities

more stack exchange communities

Stack Exchange
tour help
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.
up vote 1 down vote favorite

As you can see, the following is pretty ugly and seems to be unnecessarily long. Is there a shorter/neater way to do this?

function addNewRow(body, id){ 
    var myRow = document.createElement("tr");
    myRow.id = id+'';
    var c1 = document.createElement("td");
    var c2 = document.createElement("td");
    var c3 = document.createElement("td");
    var c4 = document.createElement("td");
    var d = document.createElement("div");
    myRow.appendChild(c1);
    myRow.appendChild(c2);
    myRow.appendChild(c3);
    myRow.appendChild(c4);
    myRow.appendChild(d);

    var i1 = document.createElement("input");
    var i2 = document.createElement("input");
    var i3 = document.createElement("input");
    var i4 = document.createElement("input");

    i1.type = 'text';
    i1.id = 'drwID_' + id;
    i1.name = 'drwID_' + id;
    i1.disabled = !editing;

    i2.type = 'text';
    i2.id = 'pos_' + id;
    i2.name = 'pos_' + id;
    i2.disabled = !editing;

    i3.type = 'text';
    i3.id = 'stm_' + id;
    i3.name = 'stm_' + id;
    i3.disabled = !editing;

    i4.type = 'text';
    i4.id = 'note_' + id;
    i4.name = 'note_' + id;
    i4.disabled = !editing;

    c1.appendChild(i1);
    c2.appendChild(i2);
    c3.appendChild(i3);
    c4.appendChild(i4);
    body.appendChild(myRow);
}
share|improve this question

1 Answer 1

up vote 1 down vote accepted

You need to use loops. Really, that's about it - all of the element creation and appending can be done in a loop, while the prefix for each of the element's name and id be stored in an array.

function addNewRow(body, id) {
    var myRow = document.createElement("tr"),
        d = document.createElement("div"),
        inputPrefix = ['drwID_', 'pos_', 'stm_', 'note_'];

    myRow.id = id + '';

    for(var i = 0; i < inputPrefix.length; i++){
        var td = document.createElement('td'),
            input = document.createElement('input');

        input.type = 'text';
        input.id = inputPrefix[i] + id;
        input.name = inputPrefix[i] + id;
        input.disabled = !editing;

        td.appendChild(input);
        myRow.appendChild(td);
    }

    myRow.appendChild(d);
    body.appendChild(myRow);
}

You are generating invalid HTML here though - trs cannot contain div elements.

share|improve this answer
    
Thanks! I didn't think of storing the prefixes in an array, that's why i couldn't figure out a way to loop it. You're right about the div too, that thing isn't supposed to be there... –  Remuladgryta Jul 4 '11 at 11:34

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.