Comparing Code Styles

2020-07-03

In a few projects in the past I made these functions

function createElem(tag, attrs = {}) { 
  const elem = document.createElement(tag);
  for (const [key, value] of Object.entries(attrs)) {
    if (typeof value === 'object') {
      for (const [k, v] of Object.entries(value)) {
        elem[key][k] = v;
      }
    } else if (elem[key] === undefined) {
      elem.setAttribute(key, value);
    } else {
      elem[key] = value;
    }
  }
  return elem;
}

function addElem(tag, parent, attrs = {}) {
  const elem = createElem(tag, attrs);
  parent.appendChild(elem);
  return elem;
}

It let's you create an element and fill the various parts of it relatively tersely.

For example

const form = addElem('form', document.body);

const checkbox = addElem('input', form, {
  type: 'checkbox',
  id: 'debug',
  className: 'bullseye',
});

const label = addElem('label', form, {
  for: 'debug',
  textContent: 'debugging on',
  style: {
    background: 'red';
  },
});

With the built in browser API this would be

const form = document.createElement('form');
document.body.appendChild(form);

const checkbox = document.createElement('input');
form.appendChild(checkbox);
checkbox.type = 'checkbox';
checkbox.id = 'debug';
checkbox.className = 'bullseye';

const label = document.createElement('label');
form.appendChild(label);
form.for = 'debug';
form.textContent = 'debugging on';
form.style.background = 'red';

Recently I saw someone post they use a function more like this

function addElem(tag, attrs = {}, children  = []) {
  const elem = createElem(tag, attrs);
  for (const child of children) {
    elem.appendChild(child);
  }
  return elem;
}

The difference to mine was you pass in the children, not the parent. This suggests a nested style like this

document.body.appendChild(addElem('form', {}, [
  addElem('input', {
    type: 'checkbox',
    id: 'debug',
    className: 'bullseye',
  }),
  addElem('label', {
    for: 'debug',
    textContent: 'debugging on',
    style: {
      background: 'red';
    },
  }),
]);

I tried it out recently when refactoring someone else's code. No idea why I decided to refactor but anyway. Here's the original code

function createTableData(thead, tbody) {
  const row = document.createElement('tr');
  {
    const header = document.createElement('th');
    header.className = "text sortcol";
    header.textContent = "Library";
    row.appendChild(header);
    for(const benchmark of Object.keys(testData)) {
      const header = document.createElement('th');
      header.className = "number sortcol";
      header.textContent = benchmark;
      row.appendChild(header);
    };
  }
  {
    const header = document.createElement('th');
    header.className = "number sortcol sortfirstdesc";
    header.textContent = "Average";
    row.appendChild(header);
    thead.appendChild(row);
    for (let i = 0; i < libraries.length; i++) {
      const row = document.createElement('tr');
      row.id = libraries[i] + '_row';
      {
        const data = document.createElement('td');
        data.style.backgroundColor = colors[i];
        data.style.color = '#ffffff';
        data.style.fontWeight = 'normal';
        data.style.fontFamily = 'Arial Black';
        data.textContent = libraries[i];
        row.appendChild(data);
        for(const benchmark of Object.keys(testData)) {
          const data = document.createElement('td');
          data.id = `${benchmark}_${library_to_id(libraries[i])}_data`;
          data.textContent = "";
          row.appendChild(data);
        };
      }
      {
        const data = document.createElement('td');
        data.id = library_to_id(libraries[i]) + '_ave__data'
        data.textContent = "";
        row.appendChild(data);
        tbody.appendChild(row);
      }
    };
  }
}

While that code is verbose it's relatively easy to follow.

Here's the refactor

function createTableData(thead, tbody) {
  thead.appendChild(addElem('tr', {}, [
    addElem('th', {
      className: "text sortcol",
      textContent: "Library",
    }),
    ...Object.keys(testData).map(benchmark => addElem('th', {
      className: "number sortcol",
      textContent: benchmark,
    })),
    addElem('th', {
      className: "number sortcol sortfirstdesc",
      textContent: "Average",
    }),
  ]));
  for (let i = 0; i < libraries.length; i++) {
    tbody.appendChild(addElem('tr', {
      id: `${libraries[i]}_row`,
    }, [
      addElem('td', {
        style: {
          backgroundColor: colors[i],
          color: '#ffffff',
          fontWeight: 'normal',
          fontFamily: 'Arial Black',
        },
        textContent: libraries[i],
      }),
      ...Object.keys(testData).map(benchmark => addElem('td', {
        id: `${benchmark}_${library_to_id(libraries[i])}_data`,
      })),
      addElem('td', {
        id: `${library_to_id(libraries[i])}_ave__data`,
      }),
    ]));
  };
}

I'm not entirely sure I like it better. What I noticed when I was writing it is I found myself having a hard time keeping track of the opening and closing braces, parenthesis, and square brackets. Effectively it's one giant expression instead of multiple individual statements.

Maybe if it was JSX it might hold the same structure but be more readable? Let's assume we could use JSX here then it would be

function createTableData(thead, tbody) {
  thead.appendChild((
    <tr>
      <th className="text sortcol">Library</th>
      (
        Object.keys(testData).map(benchmark => (
          <th className="number sortcol">{benchmark}</th>
        ))
      )
      <th className="number sortcol sortfirstdesc">Average</th>
   </tr>
  ));
  for (let i = 0; i < libraries.length; i++) {
    tbody.appendChild((
      <tr id={`libraries[i]}_row`}>
        <td style={{
          backgroundColor: colors[i],
          color: '#ffffff',
          fontWeight: 'normal',
          fontFamily: 'Arial Black',
        }}>{libraries[i]}</td>
        Object.keys(testData).map(benchmark => (
           <td id={`${benchmark}_${library_to_id(libraries[i])}_data`} />
        ))
        <td id={`${library_to_id(libraries[i])}_ave__data`} />
      </tr>
    ));
  };
}

I really don't know which I like best. I'm sure I don't like the most verbose raw browser API version. The more terse it gets though the harder it seem to be to read it.

Maybe I just need to come up with a better way to format?

I mostly wrote this post only because after the refactor I wasn't sure I was diggin it but writing all this out I have no ideas on how to fix my reservations. I did feel a little like was solving a puzzle unrelated to the task and hand to generate one giant expression.

Maybe my spidey senses are telling me it will be hard to read or edit later? I mean I do try to break down expressions into smaller parts now more than I did in the past. In the past I might have written something like

const dist = Math.sqrt((x2 - x1) ** 2 + (y2 - y1) ** 2;

but now-a-days I'd be much more likely to write something like

const dx = x2 - x1;
const dy = y2 - y1;
const distSq = dx * dx + dy * dy;
const dist = Math.sqrt(distSq);

Maybe with such a simple equation it's hard to see why I prefer spell it out. Maybe I prefer to spell it out because often I'm writing tutorials. Certainly my younger self thought terseness was "cool" but my older self finds terseness for the sake of terseness to be mis-placed. Readability, understandability, editability, comparability I value over terseness.

hmmm....🤔

Comments
OpenGL Trivia
Bad UI Design - Youtube