tchestnutt

Becky is not a programmer, but a physicist. She works in academia, alongside other scientists. Modern science generally requires some sort of heavy computation, which means scientists write code. It’s often not very good code, but that’s just the nature of the beast. The code exists to provide an analysis, not to be deployed as an app to the masses.

Most of the time. A few civil engineers were working on a brand new Android app for traffic analysis, with plans to distribute it. Unfortunately, they had some problems, and wanted more experienced eyes. Becky set aside the Fortran77 she was working on to trace through their Java code, and found this:

public class MainActivity extends Activity {
    private static double GpsLon = 0.00;
    public double getGpsLon() { return GpsLon; }
    public void setGpsLon(double value) { GpsLon = value; }
    private static boolean saveComLogeFile = true;
    public boolean getSaveComLogeFile() {return saveComLogeFile;}
    public void setSaveComLogeFile(boolean saveComLogeFile) {this.saveComLogeFile = saveComLogeFile;}
    // more than 70 similar static variables with non-static getters and setters
}

public class GpsWlan implements Runnable
{
    static MainActivity ma = new MainActivity();
    @Override
    public void run()
    {
        ma.setGpsLon(1.234);
    }
}

At this point in the article, I’d normally explain to you what this code is trying to do, and why it’s bad. Honestly though, I can’t even answer the first question. MainActivity is a megaclass of properties- and those properties are all static. That’s a fine approach for configuration settings, but you usually pair it with static getters and setters.

But the place where they actually set these variables is the really weird part of this. By implementing Runnable, they’re implying that GpsWlan should be run as its own thread- great if you’re doing heavy I/O or something, but… for setting a property? A static property? With no syncing or locking? Sure, they are setting it to a literal value, so we apparently don’t need to be too worried about race conditions, but… why?

Well, there isn’t a reason. Becky explains the process used to develop this code: “Here, Researcher N learns to program by asking Researcher N–1 for their code, learning from it and tweaking what they had.” I'm going to add a little correction: they're not learning anything from the code. This is cargo cult programming- the code they borrowed did this, so their code does it too.

[Advertisement] Manage IT infrastructure as code across all environments with Puppet. Puppet Enterprise now offers more control and insight, with role-based access control, activity logging and all-new Puppet Apps. Start your free trial today!

The hardest part is getting it out of the packaging.