tiny methods

If you had this:

public void attemptRegistration(User user) {
  if (UserTable.count() >= 1000) {
    preventRegistration(user);
  } else {
    continueRegistration(user);
  }
}

Would putting the body of that if-statement into its own method be a worthwhile refactor?

public void attemptRegistration(User user) {
  if (atMax()) {
    preventRegistration(user);
  } else {
    continueRegistration(user);
  }
}

private boolean atMax() {
  return UserTable.count() >= 1000;
}

What if that condition was more complex? Now is it worth it?

public void attemptRegistration(User user) {
  if (atMax() || registrationClosed()) {
    preventRegistration(user);
  } else {
    continueRegistration(user);
  }
}

private boolean atMax() {
  return UserTable.count() >= 1000;
}

private boolean registrationClosed() {
  return Registrar.isClosed();
}

How about combining those two private methods?

public void attemptRegistration(User user) {
  if (shouldPreventRegistration()) {
    preventRegistration(user);
  } else {
    continueRegistration(user);
  }
}

private boolean shouldPreventRegistration() {
  return UserTable.count() >= 1000 || Registrar.isClosed();
}

Or breaking it down even further?

public void attemptRegistration(User user) {
  if (shouldPreventRegistration()) {
    preventRegistration(user);
  } else {
    continueRegistration(user);
  }
}

private boolean shouldPreventRegistration() {
  return atMax() || registrationClosed();
}

private boolean atMax() {
  return UserTable.count() >= 1000;
}

private boolean registrationClosed() {
  return Registrar.isClosed();
}

What if I told you these private methods are only ever used in attemptRegistration(User)?

Does the language being used change what you think?

def attempt_registration(user)
  if prevent_registration?
    prevent_registration(user)
  else
    continue_registration(user)
  end
end

def prevent_registration?
  at_max? || registration_closed?
end

def at_max?
  UserTable.count >= 1000
end

def registration_closed?
  Registrar.closed?
end

After all, it could’ve been written in much fewer lines:

def attempt_registration(user)
  if UserTable.count >= 1000 || Registrar.closed?
    prevent_registration(user)
  else
    continue_registration(user)
  end
end

At least we can all agree on naming that magic number:

MAX_USER_COUNT = 1000

…Maybe.

I’ve committed to or abstained from all of these refactorings at points in my career, and I’ve received pushback for all such cases.

I’ve settled on what I like best though: I like tiny methods. I love tiny methods. I think of their names as terse, compiler-enforced documentation of what’s happening. They also introduce a level of indirection that gives a single point to refactor if something changes. Like imagine if prevent_registration? actually had to be this:

def prevent_registration?(user)
  at_max? || registration_closed? || blacklisted?(user)
end

Now I just look for occurrences of prevent_registration? instead of at_max? || registration_closed?. Well, hopefully no one used registration_closed? || at_max?. Even if I was in a sane (statically typed) language I wouldn’t get the benefit of the IDE’s refactoring tools, unless it was smart enough to let me specify I’m looking for these two predicates as arguments to a ||…

ugh() // ...Whatever, I'll just keep putting my comments on the same line as what they're about on so you'll criticize that instead of my tiny methods >:)