I’ve been wanting to try this for years! When you write non-trivial unit tests, you eventually end up in situations where you have to create either objects, mocks, or stubs that require a bunch of parameters, of which not all are important to the test itself. A good example of this is creation of value objects that need a couple of arguments in the constructor. For example:

ValueObject x = new ValueObject(a, b, c);
TestedObject testObject = new TestedObject();
ValueObject result = testedObject.performComplexOperation(x);
assertEquals(b + 10, result.getB());
assertEquals(0, result.getC());

A variation of this, is the n:th unused parameter to a method:
testedObject.performComplexOperation(x, 12, “a message”, -1);
Where -1 is part of the performComplexOpration method’s argument list, but totally irrelevant to the current test.

Since we do white box testing, we know which values are important, and which are not. Nonetheless, we need to supply values for everything to make compilation pass. Just setting arbitrary values (like -1 in the example above) will confuse the reader of the test. 

The solution I’ve been using so far is to have a “don’t care” final variable, which gives:

final int dontCare = 0;
ValueObject x = new ValueObject(dontCare, b, c);

or 

testedObject.performComplexOperation(x, 12, "a message", dontCare);

Another solution is commenting it, of course.

Purists will argue that this is a non-issue altogether, since we can use test-specific creation methods to hide accidental complexity of this sort. However, my experience is that tests may be difficult to read, if too much setup is delegated to other methods.

Therefore I’m going to try a middle ground by introducing an empty annotation, only applicable to local variables:

@Retention(value= SOURCE)
@Target(value= LOCAL_VARIABLE)
public @interface IgnoredByTest {
}

In the example above, its use would be:

@IgnoredByTest final int aGoodDescriptiveNameForA = 0;
ValueObject x = new ValueObject(aGoodDescriptiveNameForA, b, c);

In my world, this brings together the need of documentation with something that’s tied to the compiler, at the cost of some clutter. 

I intend to try this for a couple of months, and see how it turns out.

Mitt allra första inlägg handlade om protected static och om hur dumt det är. Nu har syndaren fått upprättelse, och protected static sin hämnd. Jag har nämligen hittat en situation där denna kombination är den enda som fungerar 

Om man ärver mellan klasser i olika paket och vill ärva ner konstanter, som man inte vill göra publika, måste man använda protected static.

Allt jag skrev i första inlägget gäller fortfarande, för objektorienteringsmässigt är det fortfarande dåligt, men ändå… Tänk att den fick bita mig i rumpan.

I min nya outtömliga källa till visdom, J2EE AntiPatterns, har jag hittar ytterligare guldkorn som på ett närgånget sätt kan relateras till mitt dagliga arbete. De har dessutom så fina namn att de förtjänar att nämnas bara för det: Mirage och Exodus! Nästan lika bra som DataVision!

Mirage är ett anti-pattern som är otroligt enkelt att förklara: man struntar i att det finns en massa färdiga verktyg för att mappa Java-objekt mot databaser (boken nämner främst CMP, men det finns ju miljoner O/R-mappers), och implementerar sitt eget ramverk i JDBC.

I några enstaka fall måste man kanske göra det, för att man vill hämta saker från flera tabeller, men här finns ju hönan- eller ägget-problematiken: Hämtar man saker från flera tabeller med handhackad JDBC för att det O/R-mappern är för dålig, eller gör man det för att man har gjort det hittills.

Nu är inte saker i verkligheten så enkla och svartvita (för mycket PL/SQL), men det är värt att komma ihåg att någon har tyckt hela förfarandet är ett anti-pattern.

Exodus(en refactoring) är mer jordnära och delvis relaterad till Mirage. Historisk evidens har visat att det är otrligt enkelt att hamna i situationer där man av en eller annan anledning tillverkar egna DTO:s (som t ex är resultatet av en JOIN) i tid och otid. Själv är jag skyldig till detta och kallar mig “agil”. Detta får dock konsekvensen att man inte direkt kan avgöra var dessa mini-väldigt-super-specifika DTO:s ska skapas. Enligt principen “Är det fult, så är det fel“, blir det väldigt smärtsamt att lägga till skapandet av dessa i en mera generell DAO, som ansvarar för ett koncept eller en tabell.

Exodus identifierar detta problem och kommer med lösningen: Allt sådant smuts ska brytas ut till en factory som använder generella DAO:s och får skapa hur dåliga och specifika DTO:s som helst. Det är inte riktigt så boken beskriver mönstret, men det är andemeningen. Jag har länge letat efter en lösning på detta problem…

När jag programmerade mot Spring Batchs klass JobParameters och anropade getString fick jag ett fett NPE. Javadocen för metoden var inte särskilt hjälpsam, så jag kollade i källkoden och hittade detta:

/**
* Typesafe Getter for the String represented by the provided key.
*
* @param key The key to get a value for
* @return The <code>String</code> value
*/
public String getString(String key){
    return parameters.get(key).toString();
}

/**
* Typesafe Getter for the String represented by the provided key. If the
* key does not exist, the default value will be returned.
*
* @param key to return the value for
* @param defaultValue to return if the value doesn't exist
* @return the parameter represented by the provided key, defaultValue
* otherwise.
*/
public String getString(String key, String defaultValue){
    if(parameters.containsKey(key)){
       return getString(key);
    }
    else{
        return defaultValue;
    }
}

Det är någonting som stör mig i den här konstruktionen. Vad är dock inte säkert. Till exempel skulle det vara helt fel att implemetera getString(key) som den är gjord nu, om den andra varianten inte fanns.

Givet att vi accepterar att vi behöver två metoder, så är det fortfarande fel att införa denna koppling. Vet inte ens något fancy name för denna typ av koppling.

Om vi nu ignorerar detta och accepterar att man måste veta namnet på sin nyckel, annars får man ett NPE (varför inte ett IllegalArgument eller IllegalState om metoden är så strikt?), så blir det fortfarande konstigt, eftersom det inte fungerar som i typ 99% av de andra fallen, och framför allt i Javas egna API.

Eller så är författarna av Spring Batch så briljanta att de vågar vägra null och det som stör är att man är då gammalmodig. Någonting stör i alla fall.

Nu är det bevisat! Läser man tillräckligt många böcker kommer man att hitta sitt favoritpattern, eftersom alla vill göra en claim for fame. Själv har jag hittat antimönstretDataVision i boken J2EE AntiPatterns.

Antimönstret gör sig synligt när databasen får bestämma designen av applikationen och visar sig via följande symptom:

Överflöd av pseudokomponenter: geggiga komponenter som bara bär en massa data.

Dysfunktionell komponentobjektmodell: överkomplicerad och svårunderhållen modell.

Ökade underhållskonstnader: det är svårt att implementera ny funktionalitet.

Fram tills jag hittade detta antimönster var mitt förtreoende för boken sådär. Nu när jag ser att det stämmer in till 200% på ett visst system jag arbetar med, så blir boken så mycket roligare att läsa.

Så glöm Singleton och kom ihåg DataVision!

Det är nästan pisamt att skriva om detta, men jag har kommit på mig själv med att göra detta mer än en gång, så nåt är knas. För att köra lite claim for fame, så har jag nog kommit på ett test-antipattern!

Problemet är pinsamt uppenbart och lösningen likaså, men ibland står det still där uppe. Problemet är föjande:

Man vill testa en metod som provocerar en annan metod till att kasta ett undantag, t ex IllegalStateException. Den testade metoden wrappar sedan undantaget och kastar ett annat. Inte helt ovanligt.

Wrapningen gör att man inte kan använda expected=IllegalStateException.class i testkoden, utan måste köra old school.

Antag att execute innehåller kod som i sin tur ger upphov till att IllegalStateException kastas:

try {
    testedClass.execute();
    fail("An exception should have been thrown!");
} catch (WrappingException e) {
    if (!(e.getCause() instanceof IllegalStateException)) {
        fail("Excepted an IllegalStateException");
    }
}

Det här testet är sjukt fult, och då kan man i enlighet med en tidigare princip dra slutsatsen att det är fel.

Lösningen är så klart extract method på det som körs i execute(), en korrekt expected= i testet för den nya metoden, och lite enklare test av execute(). Pinsamt. Det är sällan man ska behöva gå old school…

Jag fattar inte att detta inte var mitt första inlägg på bloggen! De som jobbar med mig till vardags är vid det här laget ihjältjatade av mig om den här konstruktionen.

I den kod vi jobbar med förekommer följande anti-pattern:

try {
    // massa kod som kan ge olika checked exceptions
    ...
} catch (Throwable t) {
    // Gör en rollback och logga felet
    ...
}

Jag har tjatat sönder argumentet att catch Throwable aspirerar på att fånga t ex OutOfMemoryError eller StackOverflowError; ett beteende som varierar från Java-version till Java-version (tror jag i alla fall) och som kan ge godtyckliga sidoeffekter. I detta läge kan man inte göra någonting i alla fall.

Det som dock sabbas mest av konstruktionen är mocktester med EasyMock, som kastar Error, och som då kommer att bete sig väldigt konstigt.

Slutligen… Smartare människor än jag tycker att konstruktionen är dålig.

För länge, länge sedan jobbade jag med en junior utvecklare, som vi kan kalla för Emmoth, och hoppas på att han förblir annonym. Liksom alla som är nybörjare på ett språk ville han demonstrera att han kunde Javas nyckelord, och skapade en del intressanta konstruktioner i ett redan intressant program.

När jag tröttnade på detta skrev jag ner ett antal ord på en liten whiteboard och sa att han inte får använda dem. Detta blev “Emmoths lista”. Det lustiga att är att den håller än idag! På listan stod:

static: Det finns många anledningar till att inte använda static. De främsta är enligt mig, att det möjliggör singleton och försvårar testning. Det är väldigt sällan vi på klassnivå behöver ha en “global” variabel. Statiska variabler ställer också till det i distribuerade applikationer, då vi måste hålla kolla på att de read only.

Undantaget är så klart static final och statiska klasser (men de senare är inte klockrena heller).

protected: Åter igen… Hur ofta behöver vi detta nyckelord? I 9/10 använder folk protected felaktigt istället för default access. Eftersom jag inte heller är en vän av arv, så kommer jag med argumentet att man för att korrekt använda protected måste bygga en avancerad klasshierarki, speciellt designad för utökning, men konstig på något vis. Brrrr….

synchronized: Hur många kan direkt förklara notify/wait/synchronized? Detta nyckelord är bekvämt, eftersom vi bara behöver slaska in det i en metodsignatur, så kommer det “att funka”. Självklart finns det en massa fel man kan göra: synkronisera i onödan, synkronisera över för mycket och dynkronisera på fel sak (överkurs ).

Från och med Java 5 har vid dessutom bättre sätt att synkronisera saker på.

Object: Inte lika aktuellt nu som när listan tillkom. I gamla Javan kunde man irrirera folk genom att skriva:

List myList = ...
Object o = new Object(); // För att vara säker
o = myList.get(0); // List returnerar ju Object!
MyListItem item = (MyListItem) o;

Petigt? Ja, men den som underhåller detta måste tänka ett varv till, och för att hänvisa till en nypublicerad designprincip: Är det fult så är det fel!

I jakten på tydliga variabelnamn har den så kallade “i-variabeln” alltid retat mig. I-variabeln är den vaiabel som man använder för att iterera över ett antal element. Den kan inte missförstås, eftersom den utgör ett trivialt index. T ex:

for (int i = 0; i < 10; i++) {
    System.out.println("Hello World, and i =" + i);
}

En annan variant är iteratorn:

for (Iterator i = collection.iterator(); i.hasNext(); ) {
    Item = whatever (Item) i.next();
    ...
}

Nu är det ju så att det egentligen inte är särskilt användbart att härma skollabbar och iterera över någonting i eller j antal gånger. Oftast itererar man över en collection av någonting, och behöver man göra någonting i*j gånger finns det oftast färdiga algoritmer för detta. Det finns så klart alltid undantag, men generellt sett kommer vi väldigt långt på att ersätta i-loopar med iteratorer. Dessa kan i sin tur ersättas med Java 5:s utökade for-loopar.

Det enda användningsområde som återstår är borttagning! Av någon anledning är detta ett underskattat sätt att ta bort saker på:

for (Iterator i = collection.iterator(); i.hasNext(); ) {
    if (...) {
        i.remove();

Ofta ser man folk göra en hemmasnickrad variant av borttagning och använda olika varianter som kräver i-variabeln.

Den dag Javas loopar tillåter borttagning är i-variabelns sista fäste intaget. Fram till dess – i.remove().

Final är ett av mina favoritnyckelord i Java, och empirisk erfarenhet visar att det är ett otroligt enkelt verktyg för att skapa lite tydlighet i kladdig kod med många variabler. Givetvis är det också en förutsättning för korrekt implementation av value objects, men detta är inte dagens ämne.

Dagens ämne är final i metodsignaturer. I ett kodstycke jag arbetade med idag såg jag detta (har bytt ut lite variabelnamn för att skydda den skyldige):

public FooPK create(final BarPK barPK, final int aLongParameterName, final Integer nullableParamterOne, final Integer nullableParameterTwo) { ... }

Vad gör och vad gör inte final här?

1) Det berättar att aLongParameterName inte får ändras. Som om det gjorde någon skillnad. Java skickar ändå by value, och använder man parametar som temporära variabler, så har man större problem än ett extra final.

2) Det låter metoden bo i en annonym inre klass, vilket enligt mig är det enda korrekta användningsområdet. Så är dock inte fallet här.

3) Den säger att referenserna till de tre andra parametrarna inte kommer att ändras i metoden. Åter igen… by value.

Allt detta står i en Java-bok, och är inte lika intressant som upphovsmannens förklaring: signaturen visar tydligt att parametrarna inte kommer att ändras, alltså kommunicerar metodens avsikt, typ.

Varför är man då så småsint att man atackerar en sån liten sak? Jo, från underhållsprogrammerarens perspektiv blir koden konstig. Man måste börja fundera kring varför parametrarna är final och försöka gissa upphovsmannens intention. Det är helt omöjligt att avgöra om nyckelordet ska knytas någon extra signifikans, eller om den som skrev det hela inte var säker på hur språket hanterar parametrar. Oavsett, så stjäl det fokus från det den som underhåller koden ska göra. (Plus att metodsignaturen ser ful ut i en IDE med syntax highlight).