Journal: Object Calisthenics + TDD + Advent of Code - day 2

For Practice: Object Calisthenics + TDD + Advent of Code

I'm falling into this pattern:

Red

Green asap (fake it)

Refactor (until you make it)

Refactor Express intention

Extract method, giving it a name that tells the intention, but the implementation looks more like a green asap
Change implementation to match expressed intention
Object Calisthenics rules often broken in the previous refactors:

No properties
Wrap all strings and primitives
First Class Collections

So before I did as little 'express intention' as possible.

This caused the Object Calisthenics to drive more of the changes.

Now I'm refactoring for intention, without any Object Calisthenics requiring it.

But these refactors, when done, leave a couple Object Calisthenics rules broken.

I'm now rarely triggering

7) Keep All Entities Small

Which was a mayor design driver previously

The 'until you make it' part, does feel like it can lead to less code coverage.

I don't have the following test case:

check that multiple letters fall in range

because I 'until I make it'-ed

private LetterOccurs policyEndOccurrence() {
   return new LetterOccurs(2);
}


private LetterOccurs policyStartOccurrence() {
   return new LetterOccurs(1);
}

into

private LetterOccurs policyEndOccurrence() {
   return new LetterOccurs(Integer.parseInt(policy.split(" ")[0].split("-")[1]));
}
private LetterOccurs policyStartOccurrence() {
   return new LetterOccurs(Integer.parseInt(policy.split(" ")[0].split("-")[0]));
}

without triangulation.

The acceptance test, does cover this.

But it still feels like there is something missing.

Maybe I'll figure out when I'm trying to use tests as documentation..?

In 'tests as documentation', it shows that I'm not that confident that everything works.

these clearly seem like duplicates:

when_policy_letter_found_a
when_policy_letter_found_b

And there is something missing:

Valid when_policy_range_match

I'm also wondering whether the whole Valid and Count should be separate unit tests, each testing a smaller scope.

Now I'm treating everything as a blackbox.

But that makes the tests all dependant on basic counting working.

All tests assert like this:

assertThat(counter.countValidPasswords())
      .isEqualTo(new PasswordCount(0));

assertThat(counter.countValidPasswords())
      .isEqualTo(new PasswordCount(1));

But if counting breaks, for example always returning 0.

public PasswordCount countValidPasswords() {
   int valid = 0;
   for (PasswordAndPasswordPolicy passwordAndPasswordPolicy : input) {
      if (passwordAndPasswordPolicy.isValid())
         valid++;
   }
   return new PasswordCount(0);
}

Then suddenly all Validate.* tests break also.

While actually validation still works...

Making them less blackbox, but testing the smaller units specifically solves this.

Here it is clear that something is messed up with counting, but validation is still fine.

The concern I have with testing smaller units though, is they then create friction for refactoring...

3) Wrap All Primitives And Strings

Do we need to wrap booleans?

I first wrapped them into classes. (as inner classes)

public static class LetterAtPositionMatch {
   public static final LetterAtPositionMatch MATCH = new LetterAtPositionMatch();
   public static final LetterAtPositionMatch NO_MATCH = new LetterAtPositionMatch();
}

and then later refactored it into enums

public enum LetterAtPositionMatch {
   MATCH, NO_MATCH
}

That makes sense.

I'm not too happy with the parsing that happens in static factory methods.

I'd love to "put a bad feeling into a test" like mentioned in Book: Test Driven Development: By Example - Kent Beck

But I don't think this is a case where that applies.

It also bothers me that Model: Object Calisthenics does not seem to have a problem with these one-liners with lots of hardcoded values.

Again a reason to maybe look into adding 'number of characters per line' to 7) Keep All Entities Small

One driver that seems to cause change here is "express intent" from Model: 4 rules of simple design

Integer.parseInt(policy.split(" ")[0].split("-")[1])

Maybe I should treat this as Feature Envy -> 9) No Getters/Setters/Properties

Because the string is seen as a class, and I'm accessing the string's parts directly...

I made a wrong generalisation.

In day1 the input was a string, and I kept it as a string instead of parsing it to the int it was.

And I was kind of surprised that I didn't need to parse it to an int, because there were no getters.

public class Expense implements MultiplyableExpense, SumableExpense {
   String expense;
   @Override
   public int getValueForMultiplication() {
      return Integer.parseInt(expense);
   }
   @Override
   public int getValueForSum() {
      return Integer.parseInt(expense);
   }
}

I know it looks like getters, but those are only used in the default implementation of the interface.

So in a language with multiple inheritance, I could have made both methods protected.

So nobody uses those, except it's parent class.

Back to the point I was making.

String expense;

The field was not parsed to int on creation.

And that's fine here.

But I was doing the same thing in day2,

but in a case where the string contained multiple values!

That is where I went wrong!

Whether to store

string "3"
or
int 3

is a very different case from whether to store

string "1-3 a"
or
objects

PasswordPolicyNumber 1
PasswordPolicyNumber 3
PasswordPolicyLetter "a"

I feel like the parsing should happen at one place.

And now it happens everywhere.

Every class knows how to parse itself from a string.

And putting that in static factory methods does not make it better.

I think this feeling is like the Pattern: Anti-Corruption layer from DDD.

Guarding my domain against all this ugly parsing stuff.