1
\$\begingroup\$

I am trying to simplify my code, which will do several things:

  1. Give the same width to every columns no matter how many columns the table has.
  2. Add input field if there is no text inside a td
  3. Make sure the input field is no wider than its table cell
  4. Make sure, if the input field has size attribute, the input field size will increase based on the size attribute
  5. If the table width is too narrow, increase if necessary

My HTML structure:

<table class='test' width='500'>
   <tr>
      <td>
          <span>cell 1</span>
      </td>
       <td>
          <span><input size='18'/></span>
      </td>
      <td>
          <span>cell 3</span>
      </td>
   </tr>
  <tr>
      <td>
          <span>cell 4</span>
      </td>
       <td>
          <span><input size='15'/></span>
      </td>
      <td>
          <span><input size='15'/> </span>
      </td>
   </tr>

</table>

jQuery:

$(".test td").each(function(){
        columns = $(this).parent().find('td').length;
        inputField= $(this).find('input')
        tdWidth= $(this).width() - 35;
        inputSize = $(this).find('input').attr('size');
        span = $(this).find('span');
        oldTableWdith = $(this).closest('table').attr('width');

        if($.trim($(span).html()) == '&nbsp;'){
            $(span).html("<input type='text' style='width:" + tdWidth + "px;'></input>");
        }

        inputWidth = inputSize * 11;
        inputField.css('width',inputWidth);

        if(tableWidth < (columns * inputWidth + 50)){
            tableWidth = columns * inputWidth + 50;
        }
        if(tableWidth===0){
            tableWidth = oldTableWdith
        }
        $(this).closest('table').width(tableWidth);
    })

The code works, but I feel like I can greatly reduce it and achieve what I need. I also need the span tag because I need them to do some other functions. Can anyone help me simplify the code?

\$\endgroup\$

migrated from stackoverflow.com Oct 30 '13 at 0:33

This question came from our site for professional and enthusiast programmers.

  • \$\begingroup\$ one huge efficiency improvement is only test first row for number of columns. Checking how many cells in row...for every td is a massive duplication of effort. Same with setting the table width...you are setting it every time a td is checked. If it was me I would rip whole thing apart and start with top level issues...before checking each td \$\endgroup\$ – charlietfl Oct 30 '13 at 0:30
  • \$\begingroup\$ Make sure you have tried to do all you can about widths using just CSS before you start setting widths with JavaScript. \$\endgroup\$ – 200_success Oct 30 '13 at 2:36
2
\$\begingroup\$

At first, you code looks good enough for the set of the requrements you specified. The constructions that you use are simple, so here may be place for the improvements, but this requeres developing understanding in the nontrivial subjects: CSS selectors, jQuery selectors and jQuery chaining details. I think that it may be possible to write down all the code in one chain but it may decrease maintainability.

Some recommendations:

  1. Instead of getting element html use .text().trim().
  2. Use width:100% for inputs to let CSS do the work for you, but apply it only for the inputs without size - use combination of :not and [size] for that.
  3. Tables are stretching, so just remove the table size at the end - it should be autosized to the content.
  4. Use find(...)....end() to chain the queries and separate the realizations of the requirements.
  5. Use static code analysis tools, like Jshint, jslint - it is better to add var to not interfere with the global scope and these tools let know about such kind of the errors and more.
  6. It highly unclear what is the nature of the constants (like 35, 11, etc) in the code. It is better to add at least a very small explanation on them. It greatly improves understanding and review.
\$\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.