DEV Community

Keep your code dumb

JavaScript Joel on November 15, 2017

I recently stumbled across this tweet... // Detect dark theme var iframe = document.getElementById('tweet-929851552172519424-211'); if (d...

Read full post

edA‑qa mort‑ora‑y • Nov 16 '17

I agree code shouldn't be clever, but lazy-init isn't necessarily bad, just how it was implemented here.

I use lazy initialization a lot in my current project. It never invokes genuine work though, like loading from a DB. It just creates a few new instances. The observable effect is indistinguishable from the value having always been there.

We also do lazy loading, but in a different context. And it's clearly a lazy "load", not an "init", so it's much clearer what's happening.

JavaScript Joel • Nov 16 '17

but lazy-init isn't necessarily bad, just how it was implemented here.

Totally agree. C# also has Lazy<>, what is important about C#'s Lazy<> is that it is labeled Lazy.

The main problem with this code example is that there is code that "performs work" behind a property. And properties should never "do work". That is what a function is for.

edA‑qa mort‑ora‑y • Nov 16 '17

I understand the sentiment behind "performs work", but I'm not sure it's a fixed rule. Let me give you a counter example.

At Fuse we have a UX syntax for declaring a UI, here's a really tiny made-up snippet.

<PageControl IsEnabled="false" Transition="none">
   <PurchasePage/>
   <ProfilePage/>
</PageControl>

These correlate directly to types and instances in the backing Uno/C# code. For example:

var pg = new PageControl();
pg.IsEnabled = false;
pg.Transition = TransitionType.None;

When the app starts up it constructs a UI from these obejcts -- they are live objects, forming part of the UI tree.

Now what happens if I do pg.IsEnabled = true. This is a rather complex property. In order for the control to be enabled it needs to have a few attached gestures, which install pointer handlers, and a few other things. As a programmer using our API you don't think about this much, you just expect the control to be enabled now.

It appears that in the domain of declarative programming the rule about accessors not doing work may not be valid.

JavaScript Joel • Nov 17 '17 • Edited

In this instance, I would still prefer to use something like pg.setEnabled(true).

If obj.prop was a getter that hid some logic behind it, then in the case of code like this:

// Properties get used like variables, which could put you in hot water
// if there is complex code behind the getter.
doSomething1(obj.prop);
doSomething2(obj.prop);
doSomething3(obj.prop);
Console.WriteLine(obj.prop);

We would be executing that property logic multiple times. We do not have any idea of how heavy the property is without opening up the underlying code. If each get hid a database or api call, we could quickly get into trouble.

In this instance, it would be more clear that the underlying code is doing something more than just retrieving a value...

doSomething1(obj.getProp());
doSomething2(obj.getProp());
doSomething3(obj.getProp());
Console.WriteLine(obj.getProp());

... and that the code should be written like this:

var prop = obj.getProp();
doSomething1(prop);
doSomething2(prop);
doSomething3(prop);
Console.WriteLine(prop);

I prefer this because it gives me visibility about the complexity of the code without having to dig into it. It also gives me more control over when the program performs this work.

Of course there is no set rule and situations vary and this is my personal preference :)

Cheers!

edA‑qa mort‑ora‑y • Nov 18 '17

I would argue that obj.set_enabled(true) and obj.enabled = true are semantically the same. It would be a break in the programmer's expectations if this weren't true. set/get functions are just making up for a lack of accessors/getters in a language.

Setting enabled in my scenario doesn't do something everytime. I'd never argue for that. You can safely set it repeatedly to the same value and nothing changes, only if the value changes. In declarative programming, and reactive programming though, there are a lot of dependent variables. By changing one you can set-off a cascade of modifications.

JavaScript Joel • Nov 20 '17

I am opposed to getters and setters entering into JavaScript. I don't like them in C# either.

I would argue that obj.set_enabled(true) and obj.enabled = true are semantically the same.

I still have to disagree on this for the reason that I have been bitten multiple times by badly designed software that hid logic inside a getter. This was not known to me until I experienced performance issues which required me to dig into the library I was using only to realize this was not a property value that I was expecting, but a full blown function disguised as a getter. If this had instead been a function, I would have expected some type of logic to be inside.

edA‑qa mort‑ora‑y • Nov 20 '17

Because some people misuse a feature is a bad reason to dislike it. An idiot can just as easily write a function called get_name() that deletes your hard drive.

JavaScript Joel • Nov 20 '17

Valid point.

I have been stung by this before, so I definitely hold a bias. I also prefer functional designs over OOP, so there is a lot of OOP that I also dislike.

janjuks • Jan 29 '18 • Edited

I would argue that obj.set_enabled(true) and obj.enabled = true are semantically the same.

I agree with this, but even more that setters/getters shouldn't be doing any work. I think that Joel didn't name it very good. That 'set' in method name is the part which makes them semantically the same. Maybe it's just me, but naming it obj.enable() would imply that there might be some work to be done and not just setting a value.

That being said, I have written that code myself where private variable would hold some raw data and getter would decode it only once when first needed. This post + just recently read Clean Code by Robert C. Martin reminded me of this ^_^

JavaScript Joel • Jan 30 '18

naming it obj.enable() would imply that there might be some work to be done and not just setting a value.

^ This :)

Steve Steiner • Nov 16 '17

Being the "old guy" on most projects, often I run into a piece of code written in the latest "clever" style. If I can't just read through it and explain to myself what it does, I see if I can get the person who wrote, or at least someone who thinks they can understand it, it to explain to me, in simple terms, exactly what it does.

I open a code editor, and write their explanation down, in code. Very simple code.

By the end, I understand what it does, they understand what it does, the version we've just written will usually serve as a drop-in replacement and, most importantly, they usually "get" why I write code that's so simple and not clever at all.

And, for major bonus points, when the bug is hidden in (dare I say "created by"?) the clever, it pops right out and gets solved in the course of the "simple" rewrite.

JavaScript Joel • Nov 16 '17

Totally agree!

If you have to break open up a function (or any code) to understand what it does, there is something wrong with the function.

Sergio Soares • Aug 28 '18

Great method, i will try practice as soon as possible. Thanks

Erik Skoglund • Nov 16 '17

I had the exact same scenario as you. C# Lazy loaded props that read from db, serialization for redis = bad.

But I think that specific problem has more to do about what you are trying to cache. Ideally you would just cache a poco-model instead and use interfaces for your models. Then you don't need to worry about that at all.

That's what I did in my case, mostly because of constraints on changing the lazy-loading.

That said; lazy loading can cause all kinds of unexpected weird problems.

JavaScript Joel • Nov 16 '17

The problem is so deep in the bones of the software, the company decided to just abandon the approach all together due to cost of repair.

Weston Wedding • Nov 16 '17 • Edited

I ported some logic from the GSAP animation library to C++ for use in an Arduino library that imitated the GSAP's "Tween" and "Timeline" objects in order to make animating some LEDs easier.

GSAP has some lazy initialization logic inside some of their functions and I went along with it because, hey, why not? Lazy initialization is clever and I like clever!

Unfortunately it resulted in unpredictable crashes that sent me on hours of wild goose-chasing as I went from one potential cause to another.

Eventually I realized it was the lazy initializing hiding my "real" memory usage and I was trying to allocate more memory than I had in ways that depended entirely on what I was trying to do with my library in any given program.

This would've been easy on a real computer, but you don't get stack traces or error messages in Arduino to debug with. Things just stop. Crashes would happen slower or quicker depending on how many debug messages I was using because those take up memory, too.

JavaScript Joel • Nov 16 '17

Sounds very similar to our experience. I find those hidden kind of bugs waste the most time.

Costin Manda • Dec 31 '17

I see the problem, but I don't think it's where you think it is. What you are saying is that the original code was done via lazy getters. What does that mean? It means that they started with normal properties (at least in their head) and they replaced a lot of getters with lazy instantiation. But if the getters are so similar, then what they did is basically copy paste code. I believe that is the problem.

If they would have built a lazy instantiation module, then changing the way it behaves would have been trivial, with a single point of change. I also think that is the way to refactor the code: replace all getters with a module that handles lazy instantiation and then make the single change to initialize the values at app startup when it is all working as before. Alternately, and ugly, you could go through all the objects you instantiate and call all their getters with reflection. You do use some framework to initialize your objects, right?

Lluís Josep Martínez • Nov 16 '17

It looks a lot like the lazy loading en.wikipedia.org/wiki/Lazy_loading
I use it all the time in Java and works fine. Hibernate for instance uses it at its full extent. It's clever and useful.

JavaScript Joel • Nov 16 '17

While this article uses a specific instance of lazy loading getters as an example for clever code, the article was more a commentary on clever code and not as much on lazy props.

The message I am trying to deliver here is: Clever code will yield equally clever bugs. If you keep your code dumb your bugs will be dumb as well. Dumb bugs are easier to find and squash.