1

This line in my constructor function takes 0.15ms to execute.

[].push.apply(this, selector);

Please don't tell me 0.15ms time is acceptable. I am hoping there is some faster option.

Because I think this line is converting NodeList/HTMLCollection to array. I don't necessarily need to convert it to array. This is why, I think it can be replaced with something else. Can you think of?

(function () {
    'use strict';
    
    function Query(selector) {
        if (selector.indexOf('.') !== -1) {
            selector = document.querySelectorAll(selector);
        }
        else {
            selector = document.getElementsByTagName(selector);
        }
        
        [].push.apply(this, selector);
    }
    
    function $(selector) {
        return new Query(selector);
    }
    
    
    Query.prototype = new Array;
    Query.prototype.hide = function () {
        for (var i=0,len=this.length;  i<len;  i++) {
            this[i].style.display = 'none';
        }
        return this;
    };
    
    
    window.$= $;
}());
Peter Seliger
  • 11,747
  • 3
  • 28
  • 37
samar
  • 135
  • 1
  • 8
  • 2
    You could try the `splice` equivalent or any other [`Array` method](//developer.mozilla.org/docs/Web/JavaScript/Reference/Global_Objects/Array#Static_methods) and benchmark those. All you’re trying to do is to use an instance of `Query` as if it’s an array. Essentially, it’s `this[0] = selector[0]; this[1] = selector[1]; this[2] = selector[2];`, etc., for the length of the `selector` array. Which equivalent methods have you tried? – Sebastian Simon Sep 24 '21 at 09:21
  • Not directly related to your question, but - what's with that alternation between `querySelectorAll` and `getElementsByTagName` to begin with? What's the point of that? Especially if you are not even doing it "right" - at least not, if this is supposed to be used for _any_ kind of valid selector one could think of? `#foobar` does not contain a `.`, but that doesn't mean feeding it to `getElementsByTagName` would make much sense. – CBroe Sep 24 '21 at 09:23
  • @CBroe See the OP’s [previous question](/q/69263143/4642212) for context. – Sebastian Simon Sep 24 '21 at 09:24
  • @CBroe , I know that, I will use my full code in production which contains getElementByID() etc. – samar Sep 24 '21 at 09:26
  • @SebastianSimon , I tested your suggestion. It is slightly faster. Testing further in different browsers. – samar Sep 24 '21 at 09:28
  • How about using class and spread syntax ... `class Query extends Array { constructor(selector) { this.push(...document.querySelectorAll(selector)); } hide() { /* ... */ } }` or just the spreading part within the OP's original code? ... btw, there is no need for distinguishing in between `querySelectorAll` and `getElementsByTagName`. – Peter Seliger Sep 24 '21 at 09:34
  • @PeterSeliger , I find this code "class Query extends Array { constructor" difficult to understand, than a plain function constructor & prototype that I posted here. I followed your suggestion, "[].push.apply(this, selector);" replaced with "this.push(...selector);" . It is working. Testing further if it performs faster. I also got idea from it that it can can be replaced with a for-loop but I don't it will perform faster or not. – samar Sep 24 '21 at 09:44
  • @SebastianSimon , can you please review my code answer that I just posted? What you think of for-loop like this here? will it be better than "[].push.apply(this, selector);" ? – samar Sep 24 '21 at 10:05
  • @PeterSeliger , can you please give your suggestion on my code that I just posted as an answer? – samar Sep 24 '21 at 10:09

2 Answers2

0

(function () {

  'use strict';

  class Query extends Array {

    constructor(selector) {
      super();

      this.push(...document.querySelectorAll(selector));
    }
    hide() {
      for (var idx = 0, len = this.length;  idx < len; idx++) {
        this[idx].style.display = 'none';
      }
      return this;
    }
    show() {
      for (var idx = 0, len = this.length;  idx < len; idx++) {
        this[idx].style.display = '';
      }
      return this;
    }
  }

  function $(selector) {
    return new Query(selector);
  }
  window.$ = $;

}());

const query = $('span');

setTimeout(() => query.hide(), 1000);
setTimeout(() => $('.quick-fox > span').show(), 2000);

setTimeout(() => console.log({ query }), 3000);
setTimeout(() => console.log('query.push(...$("p")) ...', query.push(...$("p"))), 4000);
setTimeout(() => console.log({ query }), 5000);
<p class="foo-bar">
  <span>foo</span>
  <span>bar</span>
  <span>baz</span>
</p>

<p class="quick-fox">
  <span>The</span>
  <span>quick</span>
  <span>brown</span>
  <span>fox</span>
</p>
Peter Seliger
  • 11,747
  • 3
  • 28
  • 37
  • what is "super();" here? – samar Sep 24 '21 at 09:49
  • it's the part which [together with `extends` ensures a proper inheritance](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Classes#sub_classing_with_extends) for each `Query` instance also being an `instanceof Array`, which means that each query instance can be operated directly with array methods (From the original code it looked like the OP wanted it this way). – Peter Seliger Sep 24 '21 at 09:55
  • can you please give your suggestion on my code that I just posted as an answer? will for-loop be better than "[].push.apply(this, selector);"? thank you for your help. – samar Sep 24 '21 at 10:10
0

Following suggestions of @SebastianSimon this[0] = selector[0]; and @PeterSeliger this.push(...selector);in comments, I thought idea of for-loop.

After some testing, I found it faster than "[].push.apply(this, selector);".

I will take more experienced developers suggestion on it.

(function () {
    'use strict';
    
    function Query(selector) {
        if (selector.indexOf('.') !== -1) {
            selector = document.querySelectorAll(selector);
        }
        else {
            selector = document.getElementsByTagName(selector);
        }
        
        //[].push.apply(this, selector);
        var i=0,  len=selector.length;
        for (;  i<len;  ) {
            this[i] = selector[i++];
        }
        this.length = len;
    }
    
    function $(selector) {
        return new Query(selector);
    }
    
    
    Query.prototype = new Array;
    Query.prototype.hide = function () {
        for (var i=0,len=this.length;  i<len;  i++) {
            this[i].style.display = 'none';
        }
        return this;
    };
    
    
    window.$= $;
}());
samar
  • 135
  • 1
  • 8
  • @SebastianSimon , What you think of for-loop like this here? will it be better than "[].push.apply(this, selector);" ? – samar Sep 24 '21 at 10:04
  • I have no further suggestions than the ones I already did provide, and as for the performance part, the OP is doing the testing, the OP decides which solution fits the requirements best. – Peter Seliger Sep 24 '21 at 10:20
  • See [How do you performance test JavaScript code?](/q/111368/4642212). – Sebastian Simon Sep 24 '21 at 10:41