loading...

Discussion on: What’s your alternative solution? Challenge #45

Collapse
miketalbot profile image
Mike Talbot

Damn I hate being a critic and you could surely have a go at a whole bunch of my code with good reason! But I thought I'd write this as a start of a discussion point if you like. While I like the long variable names in parts and the fact that it does bubble -

  • Declaring functions as const arrows is just ugly and does stuff you don't need, costing performance. I'd also say it makes it much harder to read and also means you have to be careful about the order of things, which proper functions get around. I'd leave fat arrow functions for a place where you need to capture the "this" of the outer function and not in the heart of tight loops (e.g. in a search inner loop).
  • The (i) variable in goAndRearrangeTill isn't very clear, especially as it's the "maximumIndexToSort" and one of the key principles of the algorithm. An uneducated reader might not work out what was going on there.
  • While it bubbles, it does not terminate immediately on a collection that is in order before the iterations are complete, making it far less efficient in the case data is in order compared to the original.
  • There aren't that many places that we need to work about performance and the cost of calling nested functions, but a sort loop is probably one of them. All of those calls to swapInPositions feel a little costly. They also cost 6 lines of code adding to comprehension complexity compared to 1 line to actually do the swap in place using modern JS. [arrayToSort[j], arrayToSort[j+1]] = [arrayToSort[j+1], arrayToSort[j]]
Collapse
jithinks97 profile image
Jithin KS

Wow, thanks for the response.

  • I really would like to understand how arrow function costs performance because currently, I don't use the function syntax anywhere. I consider the arrow function as the function in JS. I really need to know if I want to change that habit.

  • I'm well aware that the algorithm does not have a terminating condition if its already sorted, I just didn't want to make it more complicated.

  • Yes, I do agree about the swap, what you said is more suitable there.

Thread Thread
miketalbot profile image
Mike Talbot

So arrow functions make a class that captures the current "this" and provide it through to the inner function. It's not a massive cost, but it's additional to the cost of calling a function in both memory and instructions. It makes total sense if you need the "this" and indeed all functions declared in the body of another create a class to carry around the bound closure variables - but there is no such cost for a globally defined function.

Declaring any const requires that the code is executed before using the variable.

This works:


     doSomething() 

     function doSomething() { console.log('ok'); }

This does not:


     doSomething();
     const doSomething = ()=>console.log("Nope!");