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.
$($(this).parent().parent().parent().parent().parent().parent().children()[0]).html()
share|improve this question

closed as off-topic by Malachi, Mat's Mug, rolfl, Nikita Brizhak, ChrisWue Nov 25 '13 at 5:52

  • This question does not appear to be a code review request within the scope defined in the help center.
If this question can be reworded to fit the rules in the help center, please edit the question.

2  
This question appears to be off-topic because it is about how bad the code is. –  Malachi Nov 25 '13 at 2:44
    
@Malachi I would say it's definitely lacking proper context, but if it weren't stub code I'd vote to leave open. –  Mat's Mug Nov 25 '13 at 3:46
2  
This question appears to be off-topic because it is about stub code. See point 3 Is it actual code from a project rather than pseudo-code or example code? in our Help Center for more details. Also could use a bit more context. –  Mat's Mug Nov 25 '13 at 3:50

5 Answers 5

up vote 20 down vote accepted
.parent().parent().parent().parent().parent().parent()

This can be much simplified by using parents() which returns an array, which you can just access with an index. I.e. you can replace the above with:

.parents()[5]

The upsides of this, besides being more concise, are that it the reader of your code can see which parent you're accessing without having to count the calls to parent and that if you ever need to access a different parent, you can just change the number instead of having to add or remove calls to parent.

share|improve this answer
    
For the record, the slightly more ‘jQuery’ way to do this would be .parents().get(5). But obviously sepp2k’s suggestion is more efficient (no overhead because of an extra function call) and shorter. –  Mathias Bynens Mar 25 '11 at 12:51
    
You can also do stuff like this. .parents('form :first'); first being immediate parent to last being outermost container. –  Erik Reppen Aug 8 '12 at 23:19

This...is pretty bad. But there's no real way to tell how bad it has to be given other constraints. For instance, if this is a JS file meant to affect a page whose HTML you don't have control over, then you can't do much better than something like

$(this).parents(":eq(5)").children(":eq(0)").html()

If this is an element you have to access a lot, consider adding an id or class to it, then selecting by that.

$("#fifth-parent-first-child").html() //or something descriptive

Again, if you don't have access to the HTML, you'll need to add it yourself at runtime

$(document).ready(
     function () {
         $(foo).parents(":eq(5)").children(":eq(0)").addClass("something-descriptive")
     }
);
$(".something-descriptive").html()

Consider pasting some surrounding code for more insight.

share|improve this answer
1  
Applying a tag in the HTML is the best approach, and applying a tag dynamically is next. Specifying the number of parent elements as a number is fragile--your code will break if the page's structure is modified. –  Josh Earl Mar 14 '11 at 13:24
    
Using :eq(5) as a selector is significantly more expensive then calling .eq(5) if your not doing complex composition always use the cheaper methods instead. –  Raynos Mar 26 '11 at 17:42
    
@JoshEarl - I very much agree, I have had a lot of mysteries and headaches dealing with other's jQuery code which relies on the page structure. –  Stephen Swensen Apr 1 '11 at 15:34

Besides what sepp2k said:

Instead of accessing the DOM object of the first child and then rewrapping it in a jQuery object, you can use the method .eq() to access it directly.

So instead of this:

$($(this)/* ... */.children()[0]).html()

Use this:

$(this)/* ... */.children().eq(0).html()

And there are possibly more ways to optimize it, if you show the HTML it's operating on.

share|improve this answer
    
Thanks, that was one of the things I was looking for, very helpful :) –  Orbit Mar 12 '11 at 19:27
    
Can you combine your answers into $($(this).parents().eq(5).children().eq(0)).html()? –  David Harkness Mar 12 '11 at 22:11
    
Not quite. $(this).parents().eq(5).children().eq(0).html() See also Inaimathi's answer. –  RoToRa Mar 13 '11 at 11:34

1. Instead of doing so many .parent() calls, you can use .parents():

$(this).parents(':eq(5)');

2. You can use .eq() to get the nth occurrence of an element, you don't need to do [0] and pass it to jQuery again.

$($(this).parents(':eq(5)').children()[0]).html()

This is better:

$(this).parents(':eq(5)').children().eq(0).html();

Though jQuery has a :first-child selector, which is more understandable:

$(this).parents(':eq(5)').find(':first-child').html();

Reference

share|improve this answer

Assuming you don't have access to the html (where you could assign an id the child), the only real option you have is:

$(this).parents().eq(5).children().eq(0).html();

if you are going to be accessing the element more than once you could assign it on load:

$(document).ready(function(){
     window.foo = $(bar).parents().eq(5).children().eq(0);
});
...
foo.html();

knowing more about why you need to do dom traversing, may yield further advice.

share|improve this answer

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