4
\$\begingroup\$

I have a function I've been using for years that retrieves the index of an HTML element within its parent element, or, if a second argument is passed to it, the index of an HTML element within the collection of elements of the same name in that parent:

var childindex=function(node){
    var index=0;
    while(node.previousSibling){
        if(arguments.length==childindex.length||node.previousSibling.nodeName.toLowerCase()==arguments[1])index++;
        node=node.previousSibling;
    }
    return index;
}

However, in a recent overhaul of my JavaScript library, I went over all these old functions to try to improve upon them and this is what I came up with to replace the above:

let childindex=(node,...selectors)=>
        Array.from(selectors.length?node.parentNode.querySelectorAll(`:scope>${selectors.join(",:scope>")}`):node.parentNode.children).indexOf(node);

I'm pretty happy with the result, especially as it gives me the added ability to get the index of the element within any collection of selectors by passing each selector as a new argument and also ignores text nodes even when no additional arguments are passed to it.

I'm wondering, though, if anyone can see anywhere I can improve on it further, either in efficiency or conciseness of code.

Here's a working example to show it in action:

(()=>{
    let item=document.getElementById("getthis"),
        childindex=(node,...selectors)=>
            Array.from(selectors.length?node.parentNode.querySelectorAll(`:scope>${selectors.join(",:scope>")}`):node.parentNode.children).indexOf(node);

    //Gets index of `item` within all child elements of <section>
    console.log(childindex(item)); // 5
    //Gets index of `item` within all child <article> elements of <section>
    console.log(childindex(item,"article")); // 3
    //Gets index of `item` within all child elements of <section> with .class
    console.log(childindex(item,".class")); // 2
    //Gets index of `item` within all child <article> & <header> elements of <section>
    console.log(childindex(item,"header","article")); // 4

})();
<section>
    <header class="class"></header>
    <article></article>
    <article></article>
    <aside class="class"></aside>
    <article></article>
    <article class="class" id="getthis"></article>
    <footer></footer>
</section>

\$\endgroup\$
  • \$\begingroup\$ Having said all that, these comments are deviating from the scope of the question which is looking for improvements to rather than alternatives to the function. \$\endgroup\$ – Shaggy Nov 10 '16 at 18:39
2
\$\begingroup\$

I only use ternary operator with simple expressions.

Try to put them in independent methods. Otherwise, use the simple and boring (but expressive) if statement.

\$\endgroup\$
1
\$\begingroup\$

this piece of code is hard to read because you have a one line if statement, a lot of developers don't like this.

Something else that you are doing here is not putting spaces between operators and operands, that makes things so hard to read and goes against so many standards.

Your function:

var childindex=function(node){
    var index=0;
    while(node.previousSibling){
        if(arguments.length==childindex.length||node.previousSibling.nodeName.toLowerCase()==arguments[1])index++;
        node=node.previousSibling;
    }
    return index;
}

here is what it should look like:

var childindex = function (node) {
    var index = 0;
    while (node.previousSibling) {
        if (arguments.length == childindex.length || node.previousSibling.nodeName.toLowerCase() == arguments[1]) 
        {
            index++;
        }
        node = node.previousSibling;
    }
    return index;
}

and the new version with the Ternary could look something like this:

let childindex = (node,...selectors) =>
    Array.from(selectors.length 
            ? node.parentNode.querySelectorAll(`:scope>${selectors.join(",:scope>")}`) 
            : node.parentNode.children
        ).indexOf(node);
\$\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.