loading...
Cover image for The importance of naming

The importance of naming

lukeshiru profile image ▲ LUKE知る Updated on ・3 min read

Working with several teams in React I noticed that more often than not, the naming of properties or even components have some issues. Not the ones you'll expect from a junior developer, but something deeper.

The first day we have a class or watch a tutorial about code quality we learn that:

"You should always use meaningful names"

We all get that classic example with variables named using just one letter, like this:

let a: string;
let b: number;

And then another example of how we should name them to be meaningful:

let name: string;
let age: number;

The classic argue of the "cocky" student is that for the computer, both pieces of code are the same. And the classic response of the teacher is that for us humans a is not the same as name. I'm personally even in favor of "large names", so I'll rather see a function called loadUserConfiguration to loadConfig.

Now, the issue I noticed with React development is not so much about the length of the property names, but more about the "meaning" of those properties. Read the following code and before reading the rest of the article, try to figure out what's the issue with the naming of the properties in it:

const LoadButton = ({ getUsers, ...props }) => (
  <button onClick={getUsers} {...props}>Load</button>
);

const UserList = props => {
  const [users, setUsers] = useState<string[]>([]);
  const getUsers = () =>
    fetch("/users")
      .then(response => response.json())
      .then(fetchedUsers => setUsers(fetchedUsers));

  return (
    <div {...props}>
      <ul>{users.map(user => (
        <li>{user}</li>
      ))}</ul>
      <LoadButton getUsers={getUsers} />
    </div>
  );
};

The main issue with that piece of code is in the properties of LoadButton, to be precise, that getUsers property. Both parts of that name have some issues:

  • First: Having Users as part of the name of the property of a generic LoadButton is not generic at all. If the component was called LoadUsersButton or something similar, maybe it would make some kind of sense, but for a generic component it doesn't. You might think: "then just name it something like getData", but ...
  • Second: Having get as part of the name of that property is leaking implementation details of the parent into the children. LoadButton is not getting nothing by itself, is just a button with the text "Load" inside of it, so it should be only informing when the "load" event happens (when is clicked). The parent will then figure out what it wants to do with that "load" event. So, the fixed version could be something like:
const LoadButton = ({ onLoad, ...props }) => (
  <button onClick={onLoad} {...props}>Load</button>
);

Or even better, get rid of that all together and just use onClick:

const LoadButton = props => (
  <button {...props}>Load</button>
);

const UserList = props => {
  const [users, setUsers] = useState<string[]>([]);
  const getUsers = () =>
    fetch("/users")
      .then(response => response.json())
      .then(fetchedUsers => setUsers(fetchedUsers));

  return (
    <div {...props}>
      <ul>{users.map(user => (
        <li>{user}</li>
      ))}</ul>
      <LoadButton onClick={getUsers} />
    </div>
  );
};

Both the first and the last piece of code do the same, but the meaning of the properties in the last one is more straightforward, by avoiding unnecessary details about the implementation.

This kind of mistakes are really easy to spot. You just need to step inside a component, forget about the rest of the app, and check if all the properties it has are relevant/meaningful in its context.

That's it.
Thanks for reading!

Posted on by:

Discussion

markdown guide