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


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.


comments powered by Disqus