14
lxmcf
6y

Hmmm.... I wonder how obvious it is that JavaScript isn't my priority :-/

Comments
  • 9
    Pro tip

    for (var i=0, j=children.length; i<j; i++) {
    ...
    }
  • 2
    Pro tip for speed:

    for(var i = children.length - 1; i >= 0; i--) {
    ...
    }
  • 1
    @wizzzard that will reverse the iteration. 🙄
  • 3
    Also another improvement could be adding and removing css class. Rather than inlining style.
  • 1
    @P1Ro trust this guy. Hes right. 👌
  • 0
    @P1Ro that's the plan!
    Just pushed this out to get a base line working
  • 1
    Your javascript is pretty good. Use the performance improvement that other users suggested you.
    Just once thing (related to css), don't set the property to a void string, use "initial" instead
  • 0
    Oh and you're not removing the elements from the DOM. It's ok if you've to hide/show several times in few seconds and if you have not many elements (<500 ligjtweight), otherwise consider remove the child instead of just hide the view
  • 3
    It's very obious that js is not a priority, don't mix business logic and dom manipulation. When the template changes your code will crash horribly. And this code is a mess when you try to test it.

    Checking for > -1 isn't very readable, checking for === -1 and reverse the then-else part, is much better for understanding.

    And don't do any performance improvements, it will fuck with your maintanability. But if you do anyway, measure it at least.
  • 0
    @Divisionbyzero but why is this better?
  • 0
    @plusgut yesh I've been away from js for ages now and coming back to it isn't as easy as I remember!

    I do plan to change most of this to be more readable and minimise it as much as I can
  • 1
    @BigMacca101 Oh and what I forgot to mention: never put your state in dom-elements, this will end in a clusterfuck.
  • 1
    @mundo03 what do you mean?
  • 0
    @Divisionbyzero no, what do you mean?
  • 1
    @mundo03 you asked me why it is better. Why is WHAT better? 😅
  • 1
    @Divisionbyzero exactly! Why????
    (Messing with you)

    I am asking about your pro tip on the first comment of this rant.

    Why is declaring two variables in the first argument on the for better than what op did?
  • 4
    @mundo03 ah okay.

    Its better because it calculates the arraylength once and not on every iteration of the for loop. That boosts performance. In my opinion loops have to be good in performance!
  • 1
    Ahhhh god dammit, I did not see that!
    Clever.@Divisionbyzero
  • 1
    @Divisionbyzero oh my god you're a genius!
    I had no clue why it would be better but actually makes sense, thanks man!
  • 0
    In my opinion you all should try to avoid writing clever code, but simple maintanable code. If you cache the length of the array, you will get an 'of undefined' exception when your code grows and you delete an element.

    And the performance boost is minimal anyway. If you want to boost your performance don't use data-attributes to store your data, but a plain object.
    And if you really want to cache something, cache your getElement, dom operations are the most expansive ones.
  • 0
    @Divisionbyzero Yep that's caching the value. You write in your variable a number. If the arraylength changes, your variable won't. Since Numbers are not references in js.
  • 0
    @Divisionbyzero why did you delete your comment?
  • 2
    @plusgut because I understood your point after posting. 😅
  • 0
    @Divisionbyzero alright :)
  • 1
    @Divisionbyzero @wizzzard

    for(var i = children.length; i--;){
    // set property with shorthand if ? : ;
    }
Add Comment