Last minute geek

last minute tech news from around the net

Sunday, Dec 17th

Last update09:02:00 PM

You are here: English WTF CodeSOD: An Academic Consideration

CodeSOD: An Academic Consideration

User Rating: / 0
PoorBest 

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 aand 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!

Read all
Comment Policy:
We pre-moderate any comments and welcome all kinds of thoughts, supportive, dissenting, critical or otherwise. We delete or censor comments that are:

* abusive
* off-topic
* contain personal attacks, or against any company or organization
* promote hate of any kind
* use excessively foul language
* is blatantly spam or advertising

We do not discriminate based on the person who is posting, and we never censor comments for political or ideological reasons. We never delete an appropriate comment because we disagree with its viewpoint or ideology, and we never publish an inappropriate comment because we agree with or support its viewpoint or ideology.


Attention spammers: we manually approve all comments. Spamming and blatant advertising will NOT be published on this site and is deleted immediately, you've been warned, do not waste your time here.

Add comment

Security code
Refresh