Stupid Code: Extract complex expressions

Clever code is for ninjas. Write stupid code for stupid computers (and people)...

Clever

Every expression should be placed inline because a clever person can clearly see what's going on here:

if (
    e.clientWidth < MAX_WIDTH 
    && e.clientHeight < MAX_HEIGHT
    && isVisible({ e, isFullyVisible: true})
) { 
    // OMG so many clever :-O
}

Let's face it, if a person is too stupid to understand what this means then they have no business understanding it in the first place. Right?

Even Cleverererer

It's even better when you put everything on one line and provide a multi-line comment that shows how clever you are:

/** 
 * If the element is fully visible and its 
 * dimension are within the allowed range, 
 * then we should do something. 
 */
if (e.clientWidth < MAX_WIDTH && e.clientHeight < MAX_HEIGHT && isVisible({ e, isFullyVisible: true})) { 
   // Stupidly clever :-S
}

This way, less clever people can marvel at your logic, even though they're incapable of reasoning about it.

Average

Extracting a complex expression is only required if you have to work with average people:

const isElementInRangeAndFullyVisible = (
    element.clientWidth < MAX_WIDTH
    && element.clientHeight < MAX_HEIGHT
    && isVisible({ element, isFullyVisible: true})
);

if (isElementInRangeAndFullyVisible) {
    // Now I can has understand B-)
}

But who has time for average? Obviously, we should only accept the very best code. And this code should be a reflection this to attract only the most clever people to work on it. If an average engineer can understand your code, you're doing it wrong fool!

Stupid

Only a stupid person would need a plain text description and a stupidly specific name to understand what's happening.

/**
 * The `element` is within the max width and height and is
 * currently visible in the viewport.
 * 
 * @param element
 */
function isElementInRangeAndFullyVisible(element: HTMLElement) {
  const isWidthInRange = element.clientWidth < MAX_WIDTH;
  const isHeightInRange = element.clientHeight < MAX_HEIGHT;

  return isWidthInRange && isHeightInRange && isVisible({
    element,
    isFullyVisible: true,
  });
}

if (isElementInRangeAndFullyVisible(element)) {
    // I am five ;-P
}

I mean, who needs tool tips am I right?

That's all unless you want to read some rambling...

---

Explaining Stupid

A marginally less sarcastic explanation for the above rationale.

Rename abstact e letter variable to descriptive element.

• If we're working within the DOM, then it seems pretty clear that this is an HTML element.
• Assume that most JavaScript developers assume that e represents and event.
• If we're working with a specific type of element, we could be more explicit. Guess what these variables represent?
  • textarea
  • script
  • a
  • li

Stupid is better here because we just need to read the variable name to understand what the condition is.

If this code were part of a class / object, then it would even be better to extract each of these to accessors.

interface SomeElement {
  isHeightInRange: boolean;
  isWidthInRange: boolean;
  isInRange: boolean;
  isFullyVisible: boolean;
  isInRangeAndFullyVisible: boolean;
}

Extract complex condition to explicitly named isElementInRangeAndFullyVisible variable.

• Within the application context, it's stupidly clear what this is doing.
• If we were ever to rename this variable, it should only be to make it longer, for example:
  • wasElementInRangeAndFullyVisible
  • isElementInRangeAndFullyVisibleRightNow
  • isElementInRangeAndFullyVisibleAfterUpdatingProfile

Stupid is better here because you don't even need to worry about the content of that condition expresion. You can see that if the element is within range and is fully visible, then something is going to happen.

The only time we should worry about the condition is when that something doesn't happen, then we look at the condition and can see why.

In this case it's most likely one of two things:

• The condition is wrong so we need to update its contents to accurately reflect its name.
• The condition doesn't do what we think it does so we need to update its name to something more accurate.

Encapsulate complex condition to explicitly named function.

• Let's face it, someone stupider than you will probably need to do this exact same thing in the future.
• If you're lucky, a less stupid person might even be able to write some tests that prove your assumptions.

Stupid is better here because you have just made the implementation of your condition reusable:

if (isInRangeAndFullyVisible(header) {
  // ...
}

const visibleElements = [...elements].filter(isInRangeAndFullyVisible);

const result = isInRangeAndFullyVisible(header) ? doSomething(element) : doNothing(element);

And your stupidity has the added benefit of being testable:

it('should fail check if not in range and fully visible', () => {
  const element = document.querySelector('.not-visible');

  expect(isInRangeAndFullyVisible(element))
    .toBe(false);
});

it('should fail check if in range and partially visible', () => {
  const element = document.querySelector('.in-range.partially-visible');

  expect(isInRangeAndFullyVisible(element))
    .toBe(false);
});


it('should pass check if in range and partially visible', () => {
  const element = document.querySelector('.in-range.fully-visible');

  expect(isInRangeAndFullyVisible(element))
    .toBe(true);
});