Object Paladin

This Tumblr is about objects, and objects only.

[objectpaladin-blog]

Defining and implementing your classes piece by piece - it's TDD

I have witnessed countless of experienced programmers arguing that they don't do unit testing since they don't have monetary standard logic in their applications and thus they don't have need for strict unit testing. "We don't do bank applications." I have said this as well, but I was young and unexperienced (and idiot). Now, let's get this clear. Arguing that can mean among other things: * Unit testing is not needed if your application logic is not needed to be guaranteed to work properly? * Unit testing is not needed because your application logic is "thin"? * Unit testing is not needed because we don't have time for that? All above are total loads of bollocks. What in the earth are you doing with your applications if you're arguing that? It is the most stupid and lame excuse ever. You should never argue bollocks like that. Stop that immediately and reconsider your attitude towards your craftsmanship. I argue that: * Every single application contains complex logic * Application logic is never "thin" * You have time for unit testing. You don't have time for bloated near-death-experience code. * Applications must work correctly * This can be mastered with Test-Driven-Design Take some pride in your work and argue that "my application is *guaranteed* to work properly and if someone breaks it up I find it out instantly. I do exhaustive unit testing and it's the best shit I have ever done". Forget lame excuses. I'm not going to lie to you. Unit testing and Test-Driven-Design is hard. It's really hard. You'll have to write application after application and learn by mistake. Basic routine is really simple. We use submarine periscope class as an example (it's not "real" implementation, just a simple actual entity that can be easily modeled as a single class). Write test class defining how you think your class should be called. Ignore compilation errors! (the test-first part, at this point I usually name testing methods just "test" or something like that) public class PeriscopeTest { @Test public void test(){ Periscope periscope = new Periscope(); periscope.raise(); Assert.assertTrue(periscope.isRaised()); } } Here we have defined (defined is an important word here!) that when we have a periscope, it can be "raised", and after it is raised it must be raised (As an submarine commander I want to raise periscope to see what's happening on the outside). Now we run test for the first time and it will fail because of the compilation errors. We don't have class Periscope in our classpath. Create defined class and defined methods to make test class having no compilation errors. Eclipse quick-help is a suberb tool for this. After 15 seconds you have no compilation errors. public class PeriscopeTest { @Test public void test(){ Periscope periscope = new Periscope(); periscope.raise(); Assert.assertTrue(periscope.isRaised()); } } *** public class Periscope { public void raise() { // TODO Auto-generated method stub } public boolean isRaised() { // TODO Auto-generated method stub return false; } } Now comes the tricky part. You have to get your test pass. It's done like this: public class PeriscopeTest { @Test public void test(){ Periscope periscope = new Periscope(); periscope.raise(); Assert.assertTrue(periscope.isRaised()); } } *** public class Periscope { public void raise() { // TODO Auto-generated method stub } public boolean isRaised() { return true; } } Now wait a minute? isRaised() returns always true? WTF? It can't work like this! Yes, you're right, it can't work like this, but at the moment we have not defined test that actually does define counterpart of raising: lowering. When coding your actual class, don't ever try to look ahead. It will get you in troubles and you will find writing more tests harder and harder. This periscope example could be written by looking ahead, but when you're writing something more complex situation is quite different. Writing more test is same thing as defining new functionality. Tests should not be written for the sake of testing, but **defining**. This is basically the routine: 1) write failing test 2) make it pass and while keeping old tests passing 3) refactor, always refactor at this point (this post does consider this issue briefly, but it's done in every single step) Let's define lowering of periscope. public class PeriscopeTest { @Test public void testRaisingPeriscopeRaisesPeriscope(){ Periscope periscope = new Periscope(); periscope.raise(); Assert.assertTrue(periscope.isRaised()); } @Test public void testLoweringPeriscopeLowersPeriscope(){ Periscope periscope = new Periscope(); periscope.lower(); Assert.assertFalse(periscope.isRaised()); } } This will fail. But when we add to actual class: public class Periscope { private boolean raised = false; public void raise() { this.raised = true; } public void lower() { this.raised = false; } public boolean isRaised() { return this.raised; } } Now it passes like a charm and periscope can be raised and lowered. Next step on raising and lowering would be to write a test that checks Periscope throws an exception if one tries to lower already lowered periscope or contrary raise already raised periscope. Test frameworks have functionalities to define expected exceptions. This is important design wise since coherency and strict functionality is really important in my opinion. I mean if code "falls through" when raising already raised periscope **you never get to know that periscope using code has bugs in it, because it tries to raise already raised periscope!** But we don't go there today on code level. Let's add some more functionality. We want to turn our periscope: _As a submarine commander I want to be able to turn periscope when it's raised, to see what is happening outside_. Now quick solution for this would be to add Integer field to out Periscope class for defining degree periscope is pointed at. Integer between 0-360. This means we must write tests like "testTurningPeriscopeOver360WorksCorrectly()". Dealing with degrees is not something Periscope class should handle. This is known as single-responsibility-principle. And when you think this from the calling application **you do not want single integer defining direction periscope is pointed at. You want object defining precisely that it is a direction and it's guaranteed to be correct and coherent.** What happens in your submarine if periscope object for some reason returns 666 Integer as a direction? Yes it will fail and reason might be really hard to track. This is the most important thing in test driven design that it _forces_ programmer to think classes from the outside before they actually implement the functionality. Thinking from the outside is the cornerstone of encapsulation and object orientated design. I first did this: public class ScopeDirectionTest { private static final Integer INITIAL_DIRECTION = 100; @Test public void testConstructorSetsDirectionCorrectly(){ ScopeDirection direction = new ScopeDirection(INITIAL_DIRECTION); Assert.assertEquals(INITIAL_DIRECTION, direction.getDirection()); } @Test public void testTurningClowiseTurnsClockwise(){ ScopeDirection direction = new ScopeDirection(INITIAL_DIRECTION); Integer howMuchToTurn = 10; direction.clockwise(howMuchToTurn); Assert.assertEquals(new Integer(INITIAL_DIRECTION + howMuchToTurn), direction.getDirection()); } @Test public void testTurningCounterClocwiseTurnsCounterClockwise(){ ScopeDirection direction = new ScopeDirection(INITIAL_DIRECTION); Integer howMuchToTurn = 10; direction.counterClockwise(howMuchToTurn); Assert.assertEquals(new Integer(INITIAL_DIRECTION - howMuchToTurn), direction.getDirection()); } } *** public class ScopeDirection { private Integer direction; public ScopeDirection(Integer intitialDirection) { this.direction = intitialDirection; } public Object getDirection() { return direction; } public void clockwise(Integer howMuchToTurn) { this.direction += howMuchToTurn; } public void counterClockwise(Integer howMuchToTurn) { this.direction -= howMuchToTurn; } } And then added some more stuff for handling situation when going below 0 or over 360. public class ScopeDirectionTest { private static final Integer INITIAL_DIRECTION = 100; @Test public void testConstructorSetsDirectionCorrectly(){ ScopeDirection direction = new ScopeDirection(INITIAL_DIRECTION); Assert.assertEquals(INITIAL_DIRECTION, direction.getDirection()); } @Test public void testTurningClowiseTurnsClockwise(){ ScopeDirection direction = new ScopeDirection(INITIAL_DIRECTION); Integer howMuchToTurn = 10; direction.clockwise(howMuchToTurn); Assert.assertEquals(new Integer(INITIAL_DIRECTION + howMuchToTurn), direction.getDirection()); } @Test public void testTurningCounterClocwiseTurnsCounterClockwise(){ ScopeDirection direction = new ScopeDirection(INITIAL_DIRECTION); Integer howMuchToTurn = 10; direction.counterClockwise(howMuchToTurn); Assert.assertEquals(new Integer(INITIAL_DIRECTION - howMuchToTurn), direction.getDirection()); } @Test public void testTurnUnder0Degrees(){ ScopeDirection direction = new ScopeDirection(INITIAL_DIRECTION); direction.counterClockwise(INITIAL_DIRECTION + 1); // Now it should be 359 (100 - 101 -> 359) Assert.assertEquals(new Integer(359), direction.getDirection()); } @Test public void testTurnOverDegrees(){ ScopeDirection direction = new ScopeDirection(INITIAL_DIRECTION); direction.clockwise(261); // Now it should be 1 (100 + 261 -> 1) Assert.assertEquals(new Integer(1), direction.getDirection()); } } *** public class ScopeDirection { private Integer direction; public ScopeDirection(Integer intitialDirection) { this.direction = intitialDirection; } public Integer getDirection() { return direction; } public void clockwise(Integer howMuchToTurn) { this.direction += howMuchToTurn; if(this.direction > 360){ this.direction = this.direction - 360; } } public void counterClockwise(Integer howMuchToTurn) { this.direction -= howMuchToTurn; if(this.direction < 0) { this.direction = 360 + (this.direction); } } } And the actual class was refactored. public class ScopeDirection { private Integer direction; public ScopeDirection(Integer intitialDirection) { this.direction = intitialDirection; } public Integer getDirection() { return direction; } public void clockwise(Integer howMuchToTurn) { this.direction += howMuchToTurn; correctOverflows(); } public void counterClockwise(Integer howMuchToTurn) { this.direction -= howMuchToTurn; correctOverflows(); } private void correctOverflows() { if(this.direction > 360){ this.direction = this.direction - 360; return; } if(this.direction < 0) { this.direction = 360 + (this.direction); return; } } } Refactoring is easy stuff when you tests that assert your refactoration works correctly. This ultimately leads to clean code. Next steps would be to handle calls for turns over 360 degrees: exception or just to roll over? Now we briefly introduce ScopeDirection to our Periscope class. public class PeriscopeTest { @Test public void testRaisingPeriscopeRaisesPeriscope(){ Periscope periscope = new Periscope(); periscope.raise(); Assert.assertTrue(periscope.isRaised()); } @Test public void testLoweringPeriscopeLowersPeriscope(){ Periscope periscope = new Periscope(); periscope.lower(); Assert.assertFalse(periscope.isRaised()); } @Test public void testTurnPeriscopeClockWiseTurnsItClockwise(){ Periscope periscope = new Periscope(); // Mark initial position for asserting turn works correctly Integer initialPositionForAsserting = periscope.getScopeDirection().getDirection(); Integer howMuchToTurn = 15; periscope.turnClockwise(howMuchToTurn); Assert.assertTrue(periscope.getScopeDirection().getDirection() > initialPositionForAsserting); // Here we don't care about actual position // we just want to assert scope turns clockwise // Note: fails if turns over 360 ;) } } *** public class Periscope { private boolean raised = false; private ScopeDirection direction = new ScopeDirection(0); public void raise() { this.raised = true; } public void lower() { this.raised = false; } public boolean isRaised() { return this.raised; } public ScopeDirection getScopeDirection() { return this.direction; } public void turnClockwise(Integer howMuchToTurn) { this.direction.clockwise(howMuchToTurn); } } And from here on to eternity to defining the perfect periscope. Throw exceptions when turning lowered periscope and so on. On tests you actually most often actually define an interface even though you start by class, so at some point it's usually a bright idea to extract Periscope as an interface and introduce implementation as a SimplePeriscope or something like that. Now, you're able to implement Periscope in any way you need and you have test harnesses to test your implementation works as a periscope should! **Late note:** this is not always the case you might need to implement "fail-fast periscope" that throws exceptions if you for example want to turn lowered periscope or you might want to implement "self repairing periscope" which is raised if turned while lowered. So always test implementations but do it via interfaces. Now practise, practise, practise and practise. I do that nearly every day. Now this was object paladin's Das Boot for saturday morning while sipping caffee. I did not invent this. Book that showed me the light was Clean Code by Robert C. Martin. You can grab a copy from Amazon cheaply and I guarantee it's good read for anyone whose into programming with objects. You can read from other sources how these tests are actually run and maintained, just google. It's there. But use at least maven for builds. **STOP THE PRESS** My classes have one huge design flaw (well, at least one design flaw). Now when we ask our periscope its direction by **public ScopeDirection getDirection();** it returns of course a pointer to actual ScopeDirection which means our periscope can be turned from the outside! Periscope periscope = new Periscope(); ScopeDirection scopeDirection = periscope.getScopeDirection(); // do some mumbo-jumbo scopeDirection.clockwise(200); // and Periscope turns!! This might lead to really bad code! We should define direction instance and api to change directions separately. Maybe I'll get back to this sometime :)

[objectpaladin-blog]

Method overloading and intentional design

Most object orientated programming languages support so called method overloading, which is to declare multiple methods with similar signature (name) but varying parameters. Parameters can usually vary by type and by number. This is usually convenient for writing i.e legacy code wrappers and such, but should always be seriously considered. Take a look on a simple interface above.

public interface ThingDoerInterface { public void doYourThingWith(Integer value); public void doYourThingWith(String value); }

Interface declares two public methods. First is to do "thing" with Integer value and latter is to do "thing" with String value. Seems legit right? You get much of expression force by simply just declaring multiple methods that seem similar but actually do some magic. Yes, this can be useful, but in rather strict cases. It is useful, if "thing" is something that can be reasonably be done with Strings and Integers. Usually we see this sort of code written as legacy wrappers for changin API. Consider for example situation where we before used Integers for defining identities but now use Strings: methods can be overloaded so there is no need to re-write using code. Another thing completely is to ask why in the hell you DO NOT want to keep your codebase clean and coherent but support multiple overloaded methods for same thing.

I recently coded some stuff java.util.Timer class which is simple utility class for scheduling tasks for future execution in background thread. Timer declares four schedule(...) -methods for scheduling executions which are:

schedule(TimerTask task, Date time) 

schedule(TimerTask task, Date firstTime, long period) 

schedule(TimerTask task, long delay) 

schedule(TimerTask task, long delay, long period) 

First argument is the TimerTask to be executed. In the form that takes Date-object as a second argument method triggers execution on the time (actual unix-time, epoch time) and if it has long as a third argument it uses it as milliseconds next execution will happen. Ok, that's neat. But now, when we look two other methods we instantly see it is the delay in milliseconds that we wait until execution of task. Now these methods are incoherent in how they behave. Other one triggers execution by time epoch and other one by the delay relative to current time. Now, unfortunate programmer who is too lazy to read API or does not have source attachments might easily do something like this:

public class ExampleClass { public void exampleTimer() { Timer timer = new Timer(); Date timeToExecute = getTimeToExecute(); timer.schedule(getTimerTask(), timeToExecute.getTime()); // This would probably execute after forty years or so.. // timeToExecute.getTime() returns correponding unix-epoch } private Date getTimeToExecute() { Calendar calendar = Calendar.getInstance(); // Get your time return calendar.getTime(); } private TimerTask getTimerTask() { return new TimerTask() { @Override public void run() { // Implementation supressed } }; } }

Parameters or arguments should NEVER reveal or define any intention. This is easily fixed for more reasonable and less error prone: methods should be named differently. I came up with this:

scheduleToBeExecutedOnceOn(Date time, TimerTask task)

scheduleToBeExecutedPeriodicallyOn(Date time, long period, TimerTask task)

scheduleToBeExecutedOnceAfter(long delayInMillis, TimerTask task)

scheduleToBeExecutedPeriodicallyAfter(long delayInMillis, long period, TimerTask task)

It's not nearly perfect and I really feel this should be abstracted in different way totally. But notice how naming and ordering arguments actually make code more readable and more story like. It might not first seem as clean as API-version but when you consider your code using that class the case is totally different: you want to write a story with your code. And that is what we do: write stories someone can easily re-interpret and re-write.

It's all about intention and encapsulating. Now, go and rewrite your code.