Over a million developers have joined DZone.
{{announcement.body}}
{{announcement.title}}

Dirty Code Monday: Thinking Inside a Bigger Box

DZone's Guide to

Dirty Code Monday: Thinking Inside a Bigger Box

In this Dirty Code Monday, we throw caution to the wind and challenge our conceptions of what clean code is in this example of a production code rewrite.

· Agile Zone ·
Free Resource

Adopting a DevOps practice starts with understanding where you are in the implementation journey. Download the DevOps Transformation Roadmap. Brought to you in partnership with Techtown.

Lately, I've been thinking about how easy it is to fall into the trap of not challenging our ideas about the code we're working on. In order to challenge the default mindset of Clean Code, I recently proposed to institute Dirty Code Monday (a proposal that sort of got me into a bit of a big discussion).

Anyway, here is the report from the first successful Dirty Code Monday one week ago:

An unpromising start: I did a code review with "Programmer A"'s code on Thursday, and "Programmer H" and I had pretty much rewritten all of it. Now we had to go through the results together. We all admitted that code review is not ideal for a Dirty Code Monday, but rules are rules.

The first bit of code we came across was this:

public static void setupClientCertificate(HttpURLConnection httpURLConnection) throws IOException {
    if (isUsingClientCertificates()) {
        HttpsURLConnection httpsURLConnection = (HttpsURLConnection) httpURLConnection;
        try {
            // TODO: Find out if SSLContext.getSocketFactory is expensive an if so, cache
            httpsURLConnection.setSSLSocketFactory(createSocketFactory(keyStorePath(), keystorePassword(), trustStorePath()));
        } catch (KeyStoreException | CertificateException | NoSuchAlgorithmException | UnrecoverableKeyException | KeyManagementException e) {
            throw new RuntimeException(e);
        }
    }
}


Notice the comment?

// TODO: Find out if SSLContext.getSocketFactory is expensive an if so, cache.

I sighed. "We really have to remember not to do premature optimization, or we'll never find out if it is really slow or not." Programmer A responded "What? I thought you said this was Dirty Code Monday! Let's just do it!" And so we did. We cached the result of createSocketFactory in a static variable. Dirty Code Monday!

By now, I was starting to get my spirits up that we could actually pull this off. We glanced up at the on-wall monitor: someone had broken the build.

"Sorry," said "Programmer K". "I forgot that my commit also changed the backend."

"Programmer R" chimed in: "Yay! Your turn to get cake, K."

"Not today," said A, "Dirty Code Monday!"

Project manager K just mumbles "The cake is a lie."

I continued the code review with A. Next up was this beauty:

private boolean isValidRequest(HttpServletRequest req) {
    List<String> validPaths = req.getMethod().equals("GET") ? Configuration.validGetPaths() : Configuration.validPostPaths();
    return req.getPathInfo() != null && validPaths.stream().anyMatch(req.getPathInfo()::startsWith);
}


"Hmm..." I said, "I guess that really should deal with DELETE and PUT requests, too. But it's Dirty Code Monday." We discussed a little and A mused, "A switch-case could work here."

"Ugh! I say, "I don't like switch-statements. They kinda violate the OCP. Maybe. Maybe not here. But I still don't like it." He just looked at me and we both laughed.

private boolean isValidRequest(HttpServletRequest req) {
    if (req.getPathInfo() == null) return false;
    switch (req.getMethod()) {
        case "GET":
            return Configuration.validGetPaths().stream().anyMatch(req.getPathInfo()::startsWith);
        case "POST":
            return Configuration.validPostPaths().stream().anyMatch(req.getPathInfo()::startsWith);
        default:
            return false;
    }
}


"Ooooh," I said. "Look at that repetition of the complicated stream() expression. Perfect!" Programmer A was pleased as well. "Dirty Code Monday is pretty okay."

Perhaps you think our code wasn't that dirty. I'm actually not that embarrassed about it. But changing our perspective helped us see other ways of working with the code than we were used to. Having a phrase like "Dirty Code Monday" was a fun thing for the team to talk about. I'd like to try it again today.

Happy Dirty Code Monday, everybody!

PS: The code in this article is actual production code. The whole team has approved the quotes attributed to them.


Take Agile to the next level with DevOps. Learn practical tools and techniques in the three-day DevOps Implementation Boot Camp. Brought to you in partnership with Techtown.

Topics:
agile ,clean code ,dirty code ,code rewrite

Published at DZone with permission of

Opinions expressed by DZone contributors are their own.

{{ parent.title || parent.header.title}}

{{ parent.tldr }}

{{ parent.urlSource.name }}