January 25, 2007

Is Code Quality Subjective?

Posted in Coding, Development, Software, Style at 4:42 am by mj

Most developers I know fall into one of two camps: either they believe that their code is the best code and that everybody else who does it differently needs to learn a thing or two, or they believe that some “standard” codebase is the best code and, de facto, represents the best way to do something.

Certainly, I have a hard time understanding why anybody would write

    if (someCondition) doSomething();

rather than

    if (someCondition) {
        doSomething();
    }

Nowhere has my annoyance been higher than when I started dealing with code from a team of offshore contractors, who not only do the former (against all Java coding conventions), but they like to confuse others with beauties like:

/* is the following code active or inactive? * /
doSomething();
if (complexLogic1() && complexLogic2()) doSomethingAgain();
maybeThisCodeWorks();
/**/

and

 <!-- MyTag
   <SubTag>value</SubTag>
 </MyTag -->
 <MyTag>
   <SubTag>another value</SubTag>
 </MyTag>

Another recent example: I’ve been evaluating Solr for use at Webshots. The Solr guys have done a tremendous job producing an enterprise-y application layer on top of Lucene, and it came out of an internal CNET project by another team (developed in response, IIRC, to the acquisition of Webshots, which introduced Lucene to CNET). There are still some suspicious gaps in Solr that make it not yet suitable for Webshots’ scale, but nothing a few good contributions from us can’t tackle.

But the code style!

One good, and recent, example is negative filters. It just so happens that I solved the same issue recently at Webshots without knowledge that Solr already had an issue open on it. My solution was a NegatedFilter class that can be utilized by any consumer of Lucene, usually in conjunction with a CachingWrapperFilter. I just didn’t contribute it to Lucene (bad developer, no cookie!).

Their solution (patch available from the link) is quite Solr-specific, messy, and long. I haven’t yet compared to see if theirs is a significantly better performer, but since mine is just flipping bits and took, e.g., a 4 minute filter down to 10 seconds, I don’t see that there’s much need for improved performance.

And yet, Yonik and team will tell you that the Webshots search application is also messy and fragile, despite its scalability. (I know because I’ve had this discussion with them before. :)) In fact, they’ll also tell you that our application isn’t scalable. Which is somewhat true–but, so far, I haven’t seen where Solr is more scalable. I’ll have better performance metrics in a few weeks, though.

Finally, another example is using Spring for app configuration. The problem that I see with spring is precisely the same discussion that takes place in the Ruby community: the ability to use domain-specific languages. If you specify your own XML format for configuration, the XML file becomes a domain-specific language that feels natural for the application that you’re trying to configure. If you use Spring for your configuration, you get a lot of nicetities on the code side, but your vocabulary becomes limited to beans and properties and props, which feels natural and intuitive in only a few contexts, and certainly is nothing you’d want to give non-engineers to manage.

Thoughts, further examples or rebuttals?

Advertisements