Showing posts with label cleancode. Show all posts
Showing posts with label cleancode. Show all posts

Sunday, 30 July 2023

Can you handle the truth?

JavaScript/ECMAScript/TypeScript are officially everywhere these days and with them comes the idiomatic use of truthiness checking.

At work, recently I had to fix a nasty bug where the truthiness of an optional value was used to determine what "mode" to be in, instead of a perfectly-good enumerated type located nearby. Let me extrapolate this into a worked example that might show how dangerous this is:

 
type VehicleParameters = {
   roadSpeed: number;
   engineRPM: number;
   ...
   cruiseControlOn: boolean;
   cruiseControlSpeed: number | undefined;
}   
and imagine, running a few times a second, we had a function:

function maintainCruiseSpeed(vp: VehicleParameters) {
   const { roadSpeed, cruiseControlSpeed } = vp;
   
   if (cruiseControlSpeed ?? cruiseControlSpeed < roadSpeed) {
   	  accelerate();
   }
}

Let's suppose the driver of this vehicle hits "SET" on their cruise control stalk to lock in their current speed of 100km/h as their desired automatically-maintained speed. The control module sets the cruiseControlOn boolean to true, and copies the current value of roadSpeed (being 100) into cruiseControlSpeed

Now imagine the driver disengages cruise control, and the boolean is correctly set to false, but the cruiseControlSpeed is retained, as it is very common for a cruise system to have a RESUME feature that goes back to the previously-stored speed.

And all of a sudden we have an Unintended Acceleration situation. Yikes.

As simple as can be, but no simpler

Don't get me wrong, I like terse code; one of the reasons I liked Scala so much was the succinctness after escaping from the famously long-winded Kingdom of Nouns. I also loathe redundant and/or underperforming fields, in particular Booleans that shadow another bit of state, e.g.:

  const [isLoggedIn] = useState(false);
  const [loggedInUser] = useState(undefined);

That kind of stuff drives me insane. What I definitely really like is when we can be Javascript-idiomatic AND use the power of TypeScript to prevent combinations of things that should not be. How?

Typescript Unions have entered the chat

Let's define some types that model the behaviour we want:

  • When cruise is turned on we need target speed, there's no resume speed
  • When cruise is turned off we zero the target speed, and the resume speed
  • When cruise is set to coast (or the brake pedal is pressed) we zero the target speed, but store a resume speed
  • When cruise is turned on we need a target speed to get back to, and there's no resume speed

type VehicleParameters = {
  roadSpeed: number;
  engineRPM: number;
  cruiseControlSettings: CruiseControlSettings;
} 

type CruiseControlSettings = 
       CruiseOnSettings | 
       CruiseOffSettings | 
       CruiseCoastSettings | 
       CruiseResumeSettings

type CruiseOnSettings = {
  mode: CruiseMode.CruiseOn
  targetSpeedKmh: number;
  resumeSpeedKmh: 0;
}
type CruiseOffSettings = {
  mode: CruiseMode.CruiseOff
  targetSpeedKmh: 0;
  resumeSpeedKmh: 0;
}
type CruiseCoastSettings = {
  mode: CruiseMode.CruiseCoast
  targetSpeedKmh: 0;
  resumeSpeedKmh: number;
}
type CruiseResumeSettings = {
  mode: CruiseMode.CruiseResume
  targetSpeedKmh: number;
  resumeSpeedKmh: 0;
}

Let's also write a new version of maintainCruiseSpeed, still in idiomatic ECMAScript (i.e. using truthiness):

function maintainCruiseSpeed(vp: VehicleParameters) {
  const { roadSpeed, cruiseControlSettings } = vp;
  
  if (cruiseControlSettings.targetSpeedKmh < roadSpeed) {
      accelerate();
  }
}

And finally, let's try and update the cruise settings to an illegal combination:

function illegallyUpdateCruiseSettings():CruiseControlSettings {
  return {
    mode: CruiseMode.CruiseOff,
    targetSpeedKmh: 120,
    resumeSpeedKmh: 99,
  }
}
... but notice now, you can't; you get a TypeScript error:
Type 
'{ mode: CruiseMode.CruiseOff; targetSpeedKmh: 120; 
resumeSpeedKmh: number; }'
is not assignable to type 'CruiseControlSettings'.
  Types of property 'targetSpeedKmh' are incompatible.
    Type '120' is not assignable to type '0'

I'm not suggesting that TypeScript types will unequivocally save your critical code from endangering human life, but a little thought expended on sensibly modelling conditions just might help.

Friday, 31 January 2020

OpenHAB Broadlink Binding situation report

After #dadlife, #newjob and #otherstuff got in the way for a while last year, I got back into my role as maintainer of the OpenHAB Broadlink device binding. My first priority was to create a binding JAR that would actually work with the newly-published OpenHAB version 2.5. As the Broadlink binding is still not part of the official OpenHAB binding repository, it doesn't "automagically" get the necessary changes applied when the upstream APIs change. Luckily, it wasn't too much work.

My priorities for 2020 for this binding remain unchanged; get it to a high-quality state, make it pass the (extremely strict) linter guidelines for OpenHAB code, and get it merged into the official codebase. For me to consider it high-quality, there are still the following tasks to do:

  • Get a solid chunk of it covered by unit tests to prevent regressions; and
  • Redesign the device-discovery and identification areas of the code, to make adding new devices easier
Unit Tests

Prior to OpenHAB 2.5, bindings/addons that wished to define tests had to create an entire second project that shadowed the production code and could only be run via a strange incantation to Maven which did horrible OSGi things to run integration-style tests. Essentially, OpenHAB addons were not unit-testable by conventional means. Which given most addons are developed by unpaid volunteers, naturally meant that hardly any addons had tests.

Fortunately, one of the major changes in the 2.5 architecture has been a move towards more Java-idiomatic unit testing. Finally, classic JUnit-style unit testing with Mockito mocking will be available for fast, reliable testing within the binding. I'll be shooting for at least 60% test coverage before I'll consider submitting a PR to OpenHAB.

Discovery redesign

I've been told that new versions of some popular Broadlink devices will be arriving in 2020. In anticipation of that, I want to make it much easier to add a new device. At the moment it requires defining a new subclass of BroadlinkBaseThingHandler (which is par-for-the-course for OpenHAB, being a pretty standard Java app), but also adding "magic numbers" in a number of places to assist in looking-up and identifying devices during "discovery" and also when they boot up. I want to consolidate this such that everything needed to support a device is located within one .java file - i.e. adding support for a new device will require exactly two changes in Git:

  • The new .java file containing all the required information to support the new device; and
  • Adding a reference to this class somewhere to "pick it up".
I see no technical reason why this can't happen, and consider it only fair if maintenance of the binding will (at least partly) be a burden on the core OpenHAB team. So again, I won't be submitting this binding to become official until that work is complete.

Thanks for all the kind words from users/testers of this binding - it's very rewarding to hear people using it with great success!

Saturday, 28 December 2019

SOTSOG 2019

It's been quite a while since I last did a SOTSOG awards ceremony but there have definitely been some standouts for Standing On The Shoulders Of Giants this year. So without any further ado:

React.js

I wouldn't consider building a front-end with anything else. Hooks have been a game-changer, taking the declarative approach to the next level.

Apollo GraphQL

Version 2.x of the reference GraphQL client library brought vast improvements, and the React Hooks support fits in beautifully. Combine it with top-notch documentation and you've got a stone-cold winner for efficient and elegant data communications.

Next.js and now.sh

The dynamic duo from Zeit have almost-instantly become my go-to for insanely-quick prototyping and deployment. The built-in Lambda deployment system is ridiculously good. Just try it - it's free.

Honourable Mentions

Typescript and ES6

For making JavaScript feel like a grown-up language. Real compile-time type safety and protection from the dreaded undefined is not a function, the functional goodness of map(), reduce() and Promises without pain.

Visual Studio Code

Two Microsoft products getting a nod?!? Amazing to see - a stunning turnaround this decade from Redmond. VSCode has simply exploded onto most developers' launch bars thanks to its speed, flexibility, incredible rate of feature development and enormous plugin community. At this price point, it's very hard to look further for an IDE.

Sunday, 27 October 2019

What's in a name?

You've probably seen the old joke.
There are only 2 hard problems in Computer Science:

  1. Naming things
  2. Cache invalidation
  3. Off-by-one errors

But like every good joke there's a substantial core of truth in it. And there's a good reason why Naming Things is top of that list. It's hard. And with most geeks being complexity-addicts, we like to ladle additional difficulties on top of the naming problem; for example:

  • Compiler restrictions - Java insists on one-class-per-file, with a match between filename and class name
  • Linter rules - Requiring interfaces to start with I or implementations with Impl
  • Usability problems - if all your files are called index.js (because they live in different directories), navigating your text editor's tabs becomes a challenge
  • Readability problems - Spending too long with AbstractInjectedDAOFactoryDecorator and AbstractInjectedDAOFactoryDecoratorBuilder-type names will wear out your sanity even faster than your keyboard

But the thing is, it's worth it. Especially in the increasingly-decomposed, microservicey, lambda-esque landscape, being able to latch onto a variable and/or class because its name is well-chosen and descriptive, can be the difference between a laser-like open file by name, make change, test, commit, DONE and a flailing find in all files, open a dozen of them, trace execution path, make speculative change, get it wrong, repeat until somehow fixed loop.

Better yet, as your team grows in experience, your shared vocabulary grows with it. Those well-named classes become a shorthand; you can use them as examples or references and everyone knows what you're on about - much like the Gang of Four Design Patterns are/were all about. I say "were" because of the 23 original GoF patterns, I would argue only a handful are in common usage today and have achieved the universality of understanding the authors were hoping for: Singleton, Iterator, Observer (although probably referred to as publisher-subscriber), Adapter/Proxy (often used interchangeably) and Factory Method.

I note that the design patterns that have truly "taken off" are without exception, the "simple" ones. I mean, look at Adapter (diagrams from the Black Wasp site also linked above):


... and now compare with Abstract Factory:

All those classes. All of them need names. And in Java at least, each one living in its own file. Possibly in different directories too. Is it any wonder that people have shied away from these complex patterns?

Friday, 29 December 2017

The best feeling in software development?

I've been spending a lot of time in Javascript-land these days and while it's pretty exciting, I sure do miss some of the luxuries that really emphasise the maturity and elegance of statically-typed Scala.
Here's an example. It's incredibly-simple, but one of the most satisfying things in software development in my opinion. First, a quick definition:
trait Insertable {
  val insertedAt: Long
} 

And now, the source of all the joy:
def findMostRecentlyInserted(xs:Set[Insertable]):Option[Insertable] = {

}

Yep, that's it. An empty function. It won't even compile.

But this moment, right here, with type signatures locked down and the cursor flashing in the right spot, is where your brain can finally shift up from boilerplate mode into full power. How am I going to get from that set of candidates to the right one? What will be the fastest way? Will it read well? How should I test this?

If I've got my "good boy" hat on I'll stop here and write a few tests, which will of course fail. But sometimes, I'll just let the Scala compiler guide me - a function this simple is a great example of where strong typing really helps prevent errors. Anyone else with me on this?

Thursday, 20 April 2017

Don't Bake That Cake!

I resisted using the old "XXX Considered Harmful" riff here, but the intent is the same; learn from my pain!

I recently revisited some Scala Play Framework code I'd written a while back (circa Play 2.3) and, as is so often the case, found myself horrified at the spaghetti I had excreted. My intention had been to add some quick features to the codebase after taking it through the 2.4 and then 2.5 upgrade processes, but it was such a mess that it ended up taking several weeks (in after-hours time) to get it done.

The main culprit? The Cake Pattern

Back in the days before Play had a first-class dependency-injection mechanism, layering in traits was considered the best-practice. However, I can tell you now, with the robust DI support available via Google Guice, the Cake Pattern is definitely not a good idea.

In particular, if you're trying to favour composition over inheritance, it's best not to even start drinking the trait Kool-Aid. It's very tempting early on in a project to define what seem to be neatly-encapsulated bits of functionality, and then mix them in. At first, it seems just as elegant, if not more-so, than wiring in collaborators. The problem comes as you start to get large numbers of these mixins. Multiple-inheritance confusion, your compile time goes through the roof, testing becomes extremely awkward. Yuk. And then once you've decided you want out of the cake, you realise.

YOU CAN'T UNBAKE A CAKE

Once you have a teetering tower of inheritance, it's extremely difficult to carefully refactor it into a composed structure without the whole thing exploding. You really can't do it iteratively, and so end up with a big-bang rewrite, and your tests (if you had any) are all broken too because everything is so fundamentally different.

I was going to provide examples in this article but I'm too embarrassed and exhausted :-)

Friday, 28 October 2016

Configuring Jacoco4sbt for a Play application

Despite (or perhaps due to) my recent dalliances with React.js, I'm still really loving the Play Framework, in both pure-backend- (JSON back-and-forth) and full-stack (serving HTML) modes. It's had a tremendous amount of thought put into it, it's been rock-solid in every situation (both work- and side-project) I've deployed it, it's well-documented and there's a solid ecosystem of supporting plugins, frameworks and libraries available.

One such plugin is Jacoco4sbt, which wires the JaCoCo code-coverage tool into SBT (the build system for Play apps). Configuration is pretty straightforward, and the generated HTML report is a nice way to target untested corners of your code. The only downside (which I've finally got around to looking at fixing) was that by default, a lot of framework-generated code is included in your coverage stats.

So without further ado, here's a stanza you can add to your Play app's build.sbt to whittle down your coverage report to code you actually wrote:

jacoco.settings

jacoco.excludes in jacoco.Config := Seq(
    "views*",
    "*Routes*",
    "controllers*routes*",
    "controllers*Reverse*",
    "controllers*javascript*",
    "controller*ref*",
    "assets*"
)
I'll be putting this into all my Play projects from now on. Hope it helps someone.

Wednesday, 29 June 2016

ReactJS - Early Thoughts

It is very-much a recruiter's term but I, like many who started out with "back-end" skills, am endeavouring to be a proper Full Stack Developer.

I have always been comfortable with HTML, and first applied a CSS rule (inline) waaay back in the Netscape 3.0 days:
  <a href="..." style="text-decoration: none;">Link</a>
I'm pretty sure there were HTML frames involved there too. Good times. Anyway, the Javascript world had always been a little too wild-and-crazy for me to get deeply into; browser support sucked, the "standard library" kinda sucked, and people had holy wars about how to structure even the simplest code.

Up to about 2015, I'd been happy enough with the basic tools - JQuery for DOM-smashing and AJAX, Underscore/Lodash for collection-manipulation, and bringing in Bootstrap's JS library for a little extra polish. The whole thing based on full HTML being rendered from a traditional multi-tiered server, ideally written in Scala. I got a lot done this way.

I had a couple of brushes with Angular (1.x) along the way and didn't really see the point; the Angular code was always still layered on top of "perfectly-good" HTML from the server. It was certainly better-structured than the usual JQuery mess but the hundreds of extra kilobytes to be downloaded just didn't seem to justify themselves.

Now this year, I've been working with a Real™ Front End project - that is, one that stands alone and consumes JSON from a back-end. This is using Webpack, Babel, ES6, ReactJS and Redux as its principal technologies. After 6 weeks of working on this, here are some of my first thoughts:
  • Good  ES6 goes a long way to making Javascript feel grown-up
  • Bad The whole Webpack-Babel-bundling thing feels really rough - so much configuration, so hard to follow
  • Good React-Hot-Reloading is super, super-nice. Automatic browser reloads that keep state are truly magic
  • Bad You can still completely, silently toast a React app with a misplaced comma ...
  • Good  ... but ESLint will probably tell you where you messed up
  • Bad It's still Javascript, I miss strong typing ...
  • Good  ... but React PropTypes can partly help to ensure arguments are there and (roughly) the right type
  • Good  Redux is a really neat way of compartmentalising state and state transitions - super-important in front-ends
  • Good  The React Way of flowing props down through components really helps with code structure


So yep, there were more Goods than Bads - I'm liking React, and I'm finally feeling like large apps can be built with JavaScript, get that complete separation between back- and front-ends, and be maintainable and practical for multiple people to work on concurrently. Stay tuned for more!

Wednesday, 27 May 2015

Post-Patterns Patterns Part 2 - Partial at my house

So in my sausagefactory library, my first attempt at adding an extension mechanism was very Java-esque.
trait FieldConverter {
  def convert(a: A, b: B):Any 
}
The companion object for the CaseClassConverter supplied a default FieldConverter if you didn't need one:
object CaseClassConverter {

  def apply(converter: => FieldConverter = defaultFieldConverter)

  class DefaultFieldConverter extends FieldConverter {
     def convert(a: A, b: B):Any = {
       b
     }
  } 
}
Ye gads look at the boilerplate! All to wrap a simple function! And yet there is huge scope to still get it wrong, because if you do actually supply a custom FieldConverter, it actually ends up being you who handles all conversions from then on, even though you really don't care about most of them. So you need an if for everything to work properly:
class MyFieldConverter extends FieldConverter {

  def convert(a: A, b: B):Any = {
   if (specialCase) {
      ... // do special conversion
   } else {
      // Do the normal conversion
       b
     }
   }
}

That sucks.

So, following a nice little nugget I found in Effective Scala, I refactored the whole extension mechanism into using PartialFunctions, like this:
Make FieldConverter a type alias
  type FieldConverter = PartialFunction[(Type, Any), Any]
Chain up a user's custom FieldConverter with the default one
  val exhaustiveConverter:FieldConverter = userConverter orElse defaultConverter
Scala will check whether the userConverter is defined at a given input - if not, it'll fall through to the defaultConverter - perfect.
Now custom converters are simple case blocks
  val alwaysMakeJavaLongsIntoInts: FieldConverter = {
    case (t: Type, v: Any) if (isInt(t) && isJLong(v.getClass)) => {
      v.asInstanceOf[Long].toInt
    }
  }
A userConverter only has to worry about converting one type of thing, and doesn't know (or care) about downstream converters. A simplified Chain of Responsibility.

Wednesday, 10 September 2014

Why Clojure is fascinating me

So I've hopped aboard the Clojure boat, as it's the preferred implementation language for "new stuff" at work.

And I'm liking it. A lot. Possibly because of the way we're using it (microservices), but probably just intrinsically, it is a language that seems to fit in the head very nicely. Not encumbered by special cases, exceptions, implicit magic and overloads. (Don't worry, I still enjoy Scala, but it's a very different Kettle[Either[Throwable, Map[String, Fish]]]).

The succinctness and elegance of Clojure is also thrown into sharp relief by the other thing I seem to be spending a lot of time on at work - grinding through a multiple-hundred-thousand-line instant-legacy untested Java codebase. This thing might have been considered state-of-the-art ten years ago when it was all about 3-tiered systems putting messages on busses - iff it had been implemented nicely, but it wasn't. As a result, it's a monolithic proliferation of POJO-manipulation, with control flow by exceptions, mutable state throughout, and impossible to test in isolation.

It can take hours to find code that actually "does something", but you have to follow the path(s) all the way down from "the top" just in case there's a bug or "hidden feature" somewhere on the way through the myriad layers with methods that look like this (anonymised somewhat):
  public List getAllFoo(Integer primaryId, Short secondaryId, String detail, Locale locale,
      String timeZone, String category) {

    if (category != null) {
      Map foosMap = ParameterConstants.foosMap;
      if (foosMap != null) {
        category = (foosMap.get(category.toUpperCase()) != null) ? foosMap.get(category.toUpperCase()) : category;
      }
    }
    List values = new ArrayList();
    FooValue searchValue = new FooValue();
    List fooValues = null;
    searchValue.setPrimaryID(primaryId);
    searchValue.setSecondaryId(secondaryId);
    searchValue.setCategory(category);

    try {
      LOGGER.info(CommonAPILoggingConstants.INF_JOBTYPE_GETALL_VALIDATION_COMPLETED);
      fooValues = fooDAO.getFoos(searchValue, detail);
    } catch (FooValidationException e) {
      handleException(e.getErrorId(), e);
    } catch (Exception e) {
      throw new InternalAPIException(UNKNOWN_CODE, e);

    }
    if (FULL.equalsIgnoreCase(detail)) {
      for (FooValue fooValue : fooValues) {
        Bar bar = null;
        try {
          if (StringUtils.isNotBlank(fooValue.getBarID())) {
            bar = barDAO.getBarByBarId(fooValue.getBarID());
            fooValue.setBarName(bar.getBarName());
            fooValue.setBarShortName(bar.getShortName());

            LOGGER.debug(CommonAPILoggingConstants.DBG_JOBTYPE_GETALL_FETCH_BAR_BY_ID,
                                bar.getBarName(),fooValue.getBarID());
          }
        } catch (Exception e) {
          throw new InternalAPIException(UNKNOWN_CODE, e);
        }

        try {
          if (null != bar) {
            if (StringUtils.isNotBlank(bar.getBrandID())) {
              fooValue.setBazID(bar.getBazID());
                            Baz baz = bazDAO.getBazByBazId(fooValue.getBazID());
              LOGGER.debug(CommonAPILoggingConstants.DBG_JOBTYPE_GETALL_FETCH_BAZ,
                                    baz.getName(),fooValue.getBazID());
              fooValue.setBazName(baz.getName());
            }
          }
        } catch (Exception e) {
          throw new InternalAPIException(UNKNOWN_CODE, e);
        }

        FooValue value = filterFooDetails(fooValue);
        values.add(value);
      }
    } else if (BASIC.equalsIgnoreCase(detail)) {

      for (FooValue fooValue : fooValues) {
        FooValue value = new FooValue();
        value.setFooID(fooValue.getFooID());
        value.setJobName(fooValue.getJobName());
        value.setContentTypeName(fooValue.getContentTypeName());
        value.setCategory(fooValue.getCategory());
        value.setIsOneToMany(fooValue.getIsOneToMany());
        values.add(value);
      }
    } else {
      throw new CommonAPIException(INVALID_DETAIL_PARAM,"Detail parameter value invalid");
    }
    return values;
  }
This is everywhere. The lines that get me most annoyed are things like this:
            fooValue.setBarName(bar.getBarName());
            fooValue.setBarShortName(bar.getShortName());
These x.setFoo(y.getFoo()) stanzas can go on for tens of lines. I haven't come across a name for them, so I'll call them POJO Shuffles. They suck the will-to-live out of anyone who has to navigate them as they frequently contain misalignments, micro-adjustments and hard-coding e.g.:
            fooValue.setBarName(bar.getBazName());
            fooValue.setBarShortName("Shortname: " + bar.getShortName());
            fooValue.setBarLongName(bar.getShortName().toUpperCase());
Did you notice:
  • We're actually getting bazName from bar - almost certainly an autocomplete fail, but perhaps not?
  • The "short name" of fooValue will actually be longer than in the source object. Is that important to something?
  • There's a potential NullPointerException when we innocently try and set the "long name" of the fooValue


Then I read this gem of a paragraph from Rich Hickey, which is merely an introduction to the usage of defrecord in the official Clojure documentation, and yet reads like poetry when you've just come from code like the above:

It ends up that classes in most OO programs fall into two distinct categories: those classes that are artifacts of the implementation/programming domain, e.g. String or collection classes, or Clojure's reference types; and classes that represent application domain information, e.g. Employee, PurchaseOrder etc. It has always been an unfortunate characteristic of using classes for application domain information that it resulted in information being hidden behind class-specific micro-languages, e.g. even the seemingly harmless employee.getName() is a custom interface to data. Putting information in such classes is a problem, much like having every book being written in a different language would be a problem. You can no longer take a generic approach to information processing. This results in an explosion of needless specificity, and a dearth of reuse.
Rich Hickey

Tuesday, 26 August 2014

Fun with Scala - Post-Patterns Patterns, Part 1 - Loan Star

Are the original Software Design Patterns dead?

Seriously, aside from perhaps Builder, the dreaded Singleton, Model-View-Controller or its hipster cousin Model-View-ViewModel, when was the last time you saw one of the Gang Of Four's patterns used in a new project? Even the direct use of an Iterator is borderline bad-practice nowadays!

I'm thinking that in these days of maximal code-avoidance (and these are great days - less code is always better code in my opinion), the just amount of overhead required to implement most of these patterns is a big turn-off. It's not quite "boilerplate", that word that implies so much burden these days, but it is definitely Not Fun to churn out all those interfaces and abstract classes that do very little aside from give you that apparently-vital level of indirection which so often ends up being nothing more than a level of annoyance.

But I'm in no doubt that a new generation of post-Patterns design patterns have started to arrive, as more powerful, expressive languages enable formations of code that Gamma et al could only have dreamt of. Over the next little bit I'm going to explore a couple of nice ones that I've come across:

The Loan Pattern

... is actually the Strategy pattern but without the dreaded inheritance requirement - to refresh, here's a micro-Strategy example:
abstract class StrategySuperclass<T> {
  
  public T doSomethingIntricateInThreePartsWherePartTwoVaries() {
    T part1Result = doFirstPart();
    T part2Result = doSecondPart(part1Result);
    return doThirdPart(part2Result);
  }

  protected abstract T doSecondPart(T firstPartResult);
  ...
} 

public class ConcreteStrategyClass<T> extends StrategySuperclass<T> {
  protected T doSecondPart(T firstPartResult) {
    // Do stuff here
  }
}
The principal idea is to shield concrete classes from the complexity or intricate orchestration of resources required to do some "large" task, by allowing them to just "slot in" the specialisation or detail that they need for their solution.

The Loan Pattern does not mandate any inheritance structure at all - the two parts of the solution could be within the same file, mixed in as traits, inherited, or composed together. It is particularly excellent at protecting limited/valuable/scarce resources that have some kind of lifecycle where they should be closed/returned/de-allocated after use. Here's an example that I gave as an answer to a Stack Overflow problem related to closing resources:

Here's the loan "provider" for want of a better term:
def withPrintWriter(dir:String, name:String)(f: (PrintWriter) => Any) {
  val file = new File(dir, name)
  val writer = new FileWriter(file)
  val printWriter = new PrintWriter(writer)
  try {
    f(printWriter)
  }
  finally {
    printWriter.close()
  }
}
Which you use like this, as a "consumer":
withPrintWriter("/tmp", "myFile") { printWriter =>
  printWriter.write("all good")
}
Scala makes this kind of anonymous-function goodness really easy to both write and use. I've been using something similar in Specs2 tests recently for things like:
  • Database connections. Borrow one, give it back at the end, no matter what happened
  • Working directories. The provider makes sure the dir is empty, gives to the consumer, and then empties it out again at the end, just to be sure
  • System properties This is a really nice pattern for this hard-to-unit-test situation. Set it, call the test function, then clear it out again. Just make sure your tests are both isolated and sequential to avoid unpleasant inter-test interference

Tuesday, 6 August 2013

The elusive single FakeApplication specs2 test

As has been noted in numerous places, the Play 2 documentation on testing kinda suggests that spinning up a FakeApplication is something you can/should do in every single one of your test examples, e.g.:

  // Don't do this
class MyControllerSpec extends Specification {

  "My controller" should {

    "return a 404 on top-level GET to non-existent resource" in {
       running(FakeApplication()) {
         val home = route(FakeRequest(GET, "/mycontroller/blurg")).get
         status(home) must equalTo(NOT_FOUND)
       }
     }

     "serve up JSON on list request" in {
       running(FakeApplication()) {
         val home = route(FakeRequest(GET, "/mycontroller/list")).get
         status(home) must equalTo(OK)
         contentType(home) must beSome.which(_ == "application/json")
       }
     }
  }
}
Trust me when I say that it is not a good idea. At the very least slow tests, inconsistent test results and general Weird Things™ will happen.

What you most-likely want is something like this:
import play.api._
import play.api.test._
import org.specs2.specification._
import org.specs2.mutable._

/**
 * Mix this in to your Specification to spin up exactly one Play FakeApplication
 * that will be shut down after the last example has been run.
 * Override 'theApp' to use a customised FakeApplication
 */
trait FakePlayApplication {
  this: Specification =>

  def theApp = FakeApplication()

  def startApp = {
    System.err.println(s"Starting $theApp")
    Play.start(theApp)
  }

  def stopApp = {
    System.err.println(s"Stopping $theApp")
    Play.stop()
  }

  override def map(fs: => Fragments) = Step(startApp) ^ fs ^ Step(stopApp)
}

Which you could use in my previous example as follows:

class MyControllerSpec extends Specification with FakePlayApplication {

  "My controller" should {

    "return a 404 on top-level GET to non-existent resource" in {
         val home = route(FakeRequest(GET, "/mycontroller/blurg")).get
         status(home) must equalTo(NOT_FOUND)
     }

     "serve up JSON on list request" in {
         val home = route(FakeRequest(GET, "/mycontroller/list")).get
         status(home) must equalTo(OK)
         contentType(home) must beSome.which(_ == "application/json")
     }
  }
}
Less repetition, faster execution, and most importantly, RELIABLE TESTS!

Tuesday, 4 October 2011

Make Sure You're Testing The Right Things

I'm going to come right out and say it. I love Unit Tests.


I love mocking the collaborators with Mockito (especially the annotation-driven mode). I love the small but oh-so-valuable mini-refactors that are sometimes necessary to make a class testable. And I love watching the code coverage march inexorably to the 100% asymptote.


What makes me very sad is seeing a sorry set of unit tests, and/or a poor development environment for testing. What do I mean?


  • If you're not mocking your collaborators, you're not unit testing. And if you're not unit testing, your code is Instant Legacy Code™. You cannot properly test any non-trivial class without needing to mock its dependencies.

  • Tests should be named in terms of expected behaviour. Method name testParseIntNullString() says nothing that couldn't be gleaned from looking at the test code itself. Method name shouldThrowFormatExceptionWhenParsingNullString() tells you the intention of both the code under test and the test itself. It's documentation that won't get out of sync with the code.

  • Use the facilities provided by your testing package. I'm a TestNG fan, but recent JUnit versions are pretty good too. I use TestNG's groups option in the @Test annotation to specify the style and/or scope of each test method. Another good one is the expected or expectedException annotation option that states what is going to be thrown. Much, much tidier than having try..catch blocks in test methods.

  • Code coverage is more than just a percentage. I use the eCobertura Cobertura plugin for Eclipse while I'm writing tests - to visually indicate which parts of the code I'm actually hitting. I've lost count of the number of times it's shown me an if I thought I had covered is still "red" due to a missing precondition.