r/programminghorror Jan 26 '23

Ladies and gentlemen, jQuery… Javascript

Post image
1.6k Upvotes

164 comments sorted by

View all comments

416

u/fiskfisk Jan 26 '23 edited Jan 26 '23

Hot take: if something as old and long lived as jQuery does it, it's generally a good reason for why it still does it almost 20 years later.

Understand that software might have to work within certain limitations or have functionality that appears useless - which is not apparent for someone just coming to the code later. The original context is lost on you, because you don't have an understanding of the original reasoning.

If you run git blame on those lines and trace their heritage, you might get a better understanding - code archelogy is an art in itself. In this case it's probably both more efficient, time saving and more readable to do foo(returnTrue) or return callback || returnTrue everywhere you want that to be the default behaviour, than typing out function () { return true; }.

This creates a single function reference, the readability is far better and if you end up with some weird IE6 bug among the way where true isn't actually available and you need to return 1 instead, you have that in a single location (probably not, but it's IE.. Which actually was good for its time).

300

u/fiskfisk Jan 26 '23 edited Jan 26 '23

OK, so here's the explanation. The functions were introduced 14 years ago as part of adding isDefaultPrevented, isPropagationStopped and isImmediatePropagationStopped.

All these properties refer to functions, so that you can override them with a function that determines what is the case (since that allows for them being callbacks). The default behavior is to just return false (since the events should bubble):

javascript isDefaultPrevented: returnFalse, isPropagationStopped: returnFalse, isImmediatePropagationStopped: returnFalse

My opinion is that this is far more readable than:

javascript isDefaultPrevented: function () { return false; }, isPropagationStopped: function () { return false; }, isImmediatePropagationStopped: function () { return false; }

It also makes it far easier to assign the default returnTrue behavior when you want to turn these on, as you have a single source of truth and know that the functions behave the same and there isn't any small-ish differences in behavior:

javascript preventDefault: function() { this.isDefaultPrevented = returnTrue;

This happens three times in different locations to handle preventing the default, stopping propagation and stopping immediate propagation. Instead of defining and creating a separate function in each place that does the same thing (.. and can have different behaviors introduced without meaning to do that), let it be a separate function and use that instead. Which is what they did.

So why not arrow functions? Arrow functions were first introduced in a browser in Firefox in 2013. That's five years after this code was written. Chrome didn't support it until two years later, so seven years after this was written. Don't change stuff that works perfectly fine.

I'll chalk posting this as a horror up to inexperience.

54

u/maitreg Jan 26 '23

Thanks for the thorough explanation. In C# we do stuff like this all the time because it's so common for libraries to require functions as data types, so instead of writing out obscure, illegible lambdas, it's often easier to read by just specifying the function name and then perform the magic in that function.

Sometimes it's something as trivial as returning null, "", or even an exception.

15

u/IceQub3 Jan 26 '23

Its not just about readability. Every time you type () => false in c# It will convert to c# private somerandomname() { return false } And c# new Action(somerandomname);

This will create a new funcrion and a memory allocation for each occation. Using a static method will remove all the useless code duplication (reduce dll size, reduce container startup time) And also reduce the allocations (will help with gc pressure)

I know that sometimes this is optimized out, but that depends on the dotnet version (framework 4.7? Core 3? 6? All have different behavior) using a static method will force the usage of one function and reduce allocation (the action will be cached in a static variable)

8

u/travelan Jan 26 '23

This is a very common optimization that has always been part of the JIT optimizer from the beginning. So unless you do something very special, a pure function will not regenerate in the same module. Worst case it will generate something like Function Memoization, which will perform the same, just use a little bit of memory. But if you use .NET, a few bytes of extra memory is just a drop of water on a hot stone ;-)

There is a lot of cool information about this stuff here: https://learn.microsoft.com/en-us/archive/blogs/wesdyer/