Wednesday
18Nov2009

My opinions on multiple function / method exit points

Edited repost of this stackoverflow reply since several people have asked me about this lately.

There are good things to say about having a single exit-point, just as there are bad things to say about the inevitable "arrow" programming that results for some types of problems.

The ideal function/method should have a straight execution path with a single exit-point, no conditionals, no side effects and exactly one return point. This is a good thing to strive for, as few "wiring bugs" can hide in that kind of code; but this is not always possible or feasible.

If using multiple exit points during input validation or resource allocation, I try to put all the 'error-exits' very visibly at the top of the function in a guard clause manner.

Both the Spartan Programming article of the "SSDSLPedia" and the single function exit point article of the "Portland Pattern Repository's Wiki" have some insightful arguments around this. Also, of course, there is this post to consider.

If you really want a single exit-point (in any non-exception-enabled language) for example in order to release resources in one single place, I find the careful application of goto to be good; see for example this rather contrived example (compressed to save screen real-estate):

int f(int y) {
    int value = -1;
    void *data = NULL;

    if (y < 0)
        goto clean;

    if ((data = malloc(123)) == NULL)
        goto clean;

    /* More code */

    value = 1;
clean:
   free(data);
   return value;
}

Personally I, in general, dislike arrow programming more than I dislike multiple exit-points, although both are useful when applied correctly. The best, of course, is to structure your program to require neither. Breaking down your function into multiple chunks usually help :)

Although when doing so, I find I end up with multiple exit points anyway as in this example, where some larger function has been broken down into several smaller functions:

int g(int y) {
  value = 0;

  if ((value = g0(y, value)) == -1)
    return -1;

  if ((value = g1(y, value)) == -1)
    return -1;

  return g2(y, value);
}

Depending on the project or coding guidelines, most of the boiler-plate code could be replaced by macros. As a side note, breaking it down this way makes the functions g0, g1 ,g2 very easy to test individually.

Obviously, in an OO and exception-enabled language, I wouldn't use if-statements like that (or at all, if I could get away with it with little enough effort), and the code would be much more plain. And non-arrowy. And most of the non-final returns would probably be exceptions.

In short;

  • Few returns are better than many returns
  • More than one return is better than huge arrows, and guard clauses are generally ok.
  • Exceptions could/should probably replace most 'guard clauses' whenever possible.
Tuesday
17Nov2009

Varför vaccinerade jag mig?

Jag var idag och vaccinerade mig mot A(H1N1)v / nya influensan / svininfluensan / galtfebern, och förutsatt att jag inte drar på mig den riktiga influensan dom närmaste två veckorna (som exempelvis i väntrummet efter vaccinationen där massa människor var samlade på liten yta...barriärvård, tack? :) så bör jag vara rätt safe.

Innan jag skriver något mer kanske jag ska påpeka att jag inte har någon som helst medicinsk utbildning, så tänk själva och fråga er läkare om något är oklart.

Så varför vaccinerade jag mig? Förutom det uppenbara -- jag har ingen lust att ligga hemma och vara sjuk i flera veckor -- så är det mest en fråga om solidaritet.

Ju fler som vaccinerar sig, ju färre smittvägar har viruset, och ju färre smittvägar ju mindre risk är att personer som inte kan ta vaccinet av diverse anledningar exponeras för smittan, d.v.s populationsimmuniteten höjs. Mängden människor som inte kan ta vaccinet och mängden människor som riskerar allvarliga effekter överlappar till stor del om man får tro avsnittet "Innan du får Pandemrix" i FASS artikel om Pandemrix och frågorna 3 och 17 i ECDCs Frequently asked questions on pandemic (H1N1) 2009.

Som en bieffekt kan man hoppas att inte så många blir sjukskrivna samtidigt pga vaccinet och kan gå till jobbet och bidra till BNP som dom goda löneslavar vi är :)

Enligt mig(!) giltiga ursäkter att inte ta vaccinet

  • Graviditet
  • Äggallergi
  • Febersjuk (vilket bara är en temporär ursäkt)
  • Du har redan haft influensan
  • Rekommendation från läkare eller sköterska

Enligt mig(!) ogiltiga ursäkter

På bloggen Tankebrott görs ett mer grundligt jobb än vad jag gör i att reda ut det hela här.

För den som har intresse i att följa influensans utveckling kan jag rekommendera ECDCs site om H1N1-pandemin samt Smittskyddsinstitutets influensarapport-sida för information som inte är helt förstörd av kvällspressen.

Så har du inte vaccinerat dig än, så gå till ditt närmaste vaccinationsställe och få din skattefinansierade immunförsvarsuppgradering!

Och på vägen dit, i väntrummet och på hemresan kan jag rekommendera att lyssna på The Skeptics Guide To The Universe H1N1-special.

Thursday
24Sep2009

Trying out the picture gallery

I figured it'd be neat to be able to post pictures from time to time etc, so I tried out the photo gallery with a handful of pictures I took this spring. Try it out on the Pictures page.

 

 

Monday
21Sep2009

Overused Extract Method refactoring

I was reading the ObjectMentor blogs the other day, and came across a post called "One Thing: Extract till you Drop" by Robert C. Martin. I definitely see the point of using extract method to a great extent in order to clarify the extent of the program, but I felt that example went a bit too far for a number of reasons.

  1. The names of the extracted methods made very little sense. Even when knowing what the class does it is hard to figure out the difference between replaceAllSymbols() and replaceAllInstances() and their relations to the other methods. You actually need to read them all in order to puzzle together the call-graph.
  2. It introduces mutability in the class quite unnecessarily making reuse of the instance impossible.
  3. Some of the extracted methods have strangely asymmetric abstraction levels.

There are also some other issues regarding efficiency and thread-safeness that I reacted strongly to, especially since they are so easy to avoid.

When implementing it myself I thought about the scenarios when this would be used, and I could find two main ones. The first is to use this to iterate over different strings using the same dictionary and the second is to iterate over different dictionaries using the same string. Not knowing anything about the use-case I went for the former.

I made an attempt to improve it in the comments, but I was tired and messed it up a bit. Normally I probably would have solved the same problem in a manner closer to this:

public class SymbolReplacer {
    private static final Pattern pattern =
        Pattern.compile("\\$([a-zA-Z]\\w*)");

    private final SymbolLookup lookup;

    SymbolReplacer(SymbolLookup lookup) {
        this.lookup = lookup;
    }

    public String replace(CharSequence stringToReplace) {
        final Matcher matcher = pattern.matcher(stringToReplace);
        return buildReplacement(matcher).toString();
    }

    private StringBuffer buildReplacement(Matcher matcher) {
        final int bufferLength =
            matcher.regionEnd() - matcher.regionStart();
        final StringBuffer buffer =
            new StringBuffer(bufferLength);

        fillBuffer(matcher, buffer);
        return buffer;
    }

    private void fillBuffer(Matcher matcher, StringBuffer buffer) {
        while (matcher.find())
            matcher.appendReplacement(buffer,
                getMatchReplacement(matcher));
        matcher.appendTail(buffer);
    }

    public String getMatchReplacement(Matcher matcher) {
        final String expansion =
            lookup.lookupValue(matcher.group(1));
        return (expansion != null) ?
            expansion :
            Matcher.quoteReplacement(matcher.group(0));
    }
}

I introduced the interface SymbolLookup as a replacement for the secret getSymbol() method. There are a few things I would like to fix in this as well; the getMatchReplacement() method is not as clear as I would have liked, and the line of abstraction between the different methods/layers is not well defined. Still, provided that SymbolLookup is thread-safe, so is this class. Also, you can reuse the instance to iterate over many strings.

Sunday
20Sep2009

Some info

Just trying out some new software. This page may or may not stay at the same place (probably not), and may or may not continue to be updated and may or may not continue to exist for long.