I have an HTML table representing a database table and a sidebar which holds different anchors ready to perform CRUD operations on click.

Because the hrefs in the anchors aren't general valid for all columns I have written a script which automatically inserts the id of a selected column into the hrefs:

/localhost/image/var/edit => /localhost/image/4/edit

The code I have written works but it isn't very clean and smart. Could somebody take a look and say where I have space of improvement?

<script type="text/javascript" src="{{ asset('bundles/acme/js/jquery.js'></script>
<script type="text/javascript">
$('table tr').click(function(){
    var name = $(this).find('td:nth-child(2)').html();

    $('.image_bar ul li a').each(function(){
        var oldUrl = $(this).attr('href');
        var newUrl = oldUrl.replace('var', name);
        $(this).attr('href', newUrl);
  • \$\begingroup\$ fiddle please.. \$\endgroup\$ – gdoron Apr 29 '12 at 13:53
  • 1
    \$\begingroup\$ Your code is fine, only thing i would change is giving the table an ID, so it dosent bind all tables, and it find the element faster. And combine oldUrl and newUrl into one line, no reason to make the oldurl since you don't use it as an var. \$\endgroup\$ – Marco Johannesen Apr 29 '12 at 14:27

I do not like too much this:

var name = $(this).find('td:nth-child(2)').html(); 

I would generally use html5 data attribute on the tr row to pass the id.


   <tr data-id="1"><td>bla</td><td>bla</td><td>bla</td></tr>
   <tr data-id="2"><td>bla</td><td>bla</td><td>bla</td></tr>

var selectedId = $(this).data("id"); 
  • \$\begingroup\$ Hi, the problem is that I don't directly use the id as identificator. I mostly use a slug which is in some <td>-block. But good idea for normal identification! \$\endgroup\$ – user1246987 Apr 29 '12 at 16:38

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.