Kengo's blog

Technical articles about original projects, JVM, Static Analysis and TypeScript.

When we need to divide small class into 2 classes

3~4 years ago, I divided code into some classes which has own responsibility. Now, I have got another option, dividing into some classes which has 1 processing.

1 class which has 1 responsibility and 2 processing

Imagine that how to code a method which returns true if given String contains letter. We need to handle surrogate-pair correctly, so we need code to detect it. I think normal way for old me is making a class. For instance,

class CharSequenceWrapperToHandleSurrogatePair {
  private final CharSequence inner;
  CharSequenceWrapperToHandleSurrogatePair(CharSequence wrapped) {
    inner = checkNotNull(wrapped);
  }

  boolean containsLetter() {
    for (int i = 0; i < inner.length(); ++i) {
      // extracting this part to private `peekCodePoint` method is not good.
      // we need to pass `i`, and this method should increment it.
      int codePoint;
      if (Character.isHighSurrogate(inner.charAt(i))) {
        codePoint = (inner.charAt(i) << 16) + inner.charAt(i + 1);
        ++i;
      } else {
        codePoint = inner.charAt(i);
      }
      if (Character.isLetter(codePoint)) {
        return true;
      }
    }
    return false;
  }
}

I guess that some of you think that this class has 2 responsibilities. I do not think so, picking up codepoint is a part of responsibility like loop, throwing exception and precondition check. But your opinion might be correct. The definition of 'processing' is not clear, just feeling.

2 classes which has 1 responsibility and 1 processing

Now I have another option: dividing contains letter method into 2 class, which has responsibility to 'peek next codepoint' and 'judge codepoint is letter or not'. In this case, each class has clear responsibility and processing. You can read and understand them in short time.

class LetterFinder {
  boolean findLetter(String target) {
    CodepointIterator iterator = new CodepointIterator(target);
    while (iterator.hasNext()) {
      int codepoint = iterator.next().intValue();
      if (Character.isLetter(codepoint)) {
        return true;
      }
    }
    return false;
  }
}

class CodepointIterator implements Iterator<Integer> {
  private CharSequence sequence;
  private int index;

  CodepointIterator(CharSequence sequence) {
    this.sequence = sequence;
  }

  @Override
  public boolean hasNext() {
    return index < sequence.length();
  }

  @Override
  public Integer next() {
    final int result;
    if (Character.isHighSurrogate(sequence.charAt(index))) {
      result = (sequence.charAt(index) << 16) + sequence.charAt(index + 1);
      index += 2;
    } else {
      result = sequence.charAt(index);
      index += 1;
    }
    return Integer.valueOf(result);
  }

  // other code is omitted
}

1st way is enough good, but 2nd way also has own merit

New code is longer than old one, so sometimes it is bad to use. Using simple solution is always correct. But new one is more testable than old one, because each class has only one processing... it is better than utility class. And it is good to handle sequential data by iterator, because it is flexible to use, and easy to optimize performance. Please imagine that how to implement lazy loading or buffering for large data source, what we have to do is just wrap source by another iterator.

In short, we can use 2nd way when:

  • we want to share code which do some processing
  • we have a plan to optimize it