I’d like to take you on a journey with me.

Many years ago I was fiercely into coding standards. All indents must be tabs (I can hear the cringing). All curly braces open on the same line (blasphemy!). etc etc. But over time I started caring less and less and I’m at a point now where I hardly notice. This would all be fine and good if I lived in a vacuum and didn’t work with other people. But the way things shake out I do and this has lead to some strange interactions.

I’ve now worked in enough languages and on enough teams that I can’t keep one set of conventions separate from another. I got in the habit of spending time up front configuring my IDE to do this for me. This works some of the time. When I create a new class, for instance, it puts the braces on the correct lines and inserts the correct amount of white space between blah blah blah and so forth. When I’m typing I have certain habits that are ingrained but I can usually fix that with ctrl+shift+F or whatever the shortcut my IDE uses for formatting. But even that can’t catch everything as I often forget to do it. Also, it can’t handle every rule that someone, somewhere came up with.

I used to work at a place with strict coding standards. And when I say strict, well, man, it was pretty serious. Code reviews were used to enforce these standards. There was also a weird rule that someone else couldn’t fix broken style in your code. So if you missed a line break you would fail the code review and have to go over it and submit again. This was kind of an extreme case so I don’t want to use it to build my story.

Another place I worked did not enforce standards or make a big deal about them but they still existed. I’d do my best to setup my IDE to take care of it but transgressions would slip through. The code would always get merged (barring any kind of logic error or design flaw) but later on a separate ticket would be opened where someone “fixed” the style faux pas and generally added me and a few others as a reviewer. Lets think about that for a second. At most places nowadays the general work flow is to open a “ticket” or “task” which the work is tracked against. Next the repository is forked or branched, checked out into someones work environment, and then the code style is cleaned up. Maybe manually or maybe they have their IDE handle it. Next a pull request or patch is generated and people are “tagged” to review it. Lets say 3 people. These 3 people have to look over the code and give their approval. Say it takes each an average of 10 minutes to look over the changes, become familiar with the problem domain, make sure bugs weren’t introduced, check out the code and run any tests. To do this they have to stop what they are currently doing or wait until they get to a break point. Either way they were probably notified via some automated system that there was something they had to do. They will need to spend a certain amount of time switching gears into this task. Once they are done they will need to switch gears back to what they were working on before. This task switching comes at a HUGE cost. Programmers, by their very nature, are not good at task switching.

Conservatively, 1 hour is spent per reviewer plus another hour for the programmer who did the task. 4 hours total. Depending on where you are prices and participation may vary but once you factor in salaries, bonuses, stock options and other overhead (HR, rent, electricity, health insurance, perks) we get an average of about $200/hr times 4 hours = $800. Nearly a grand was just dropped to tidy up the style in a handful of classes.

So why the compulsion toward coding standards? It’s obviously important to many people. It was to me at one point. So what changed? Why did I stop caring? Let’s look at some code.

Say you’ve got a gigantic method which mixes concerns and levels of abstraction. Something like this:

public class CouponEmailer {
    
    public void sendCouponsTOCustomers() {
        SQLUtils.openConnection(SQLUtils.CUSTOMER_DB);
        
        
     List<map<string, object="">> Query = SQLUtils.query("SELECT * FROM CUSTOMER WHERE COUPON_OPT_IN=1");
        
        SQLUtils.closeConnection(SQLUtils.CUSTOMER_DB);
        
               SQLUtils.openConnection(SQLUtils.COUPON_DB);
        
                            for (Map<string, object=""> customer_map : Query) {
            Long numberOfCoupons =
                (Long)customer_map.get("number_of_coupons");
            
            for (int i = 0; i < numberOfCoupons; i++)
            {
    List<map<string, object="">> query1 = SQLUtils.query("SELECT * FROM COUPON ORDER BY RANDOM LIMIT 1");

    MailUtil.sendEmail((String)customer_map.get("email"), (String)query1.get(0).get("content"));
}
}

SQLUtils.closeConnection(SQLUtils.COUPON_DB);
}
    
}

Gross, huh? Lets hit ctrl+f and get that indentation all rowing together and rename some of those methods as they’re using a mixture of camel, Pascal and underscores:

public class CouponEmailer {

    public void sendCouponsToCustomers() {
        
        SQLUtils.openConnection(SQLUtils.CUSTOMER_DB);

        List<map<string, object="">> query = SQLUtils.query("SELECT * FROM CUSTOMER WHERE COUPON_OPT_IN=1");

        SQLUtils.closeConnection(SQLUtils.CUSTOMER_DB);

        SQLUtils.openConnection(SQLUtils.COUPON_DB);

        for (Map<string, object=""> customerMap : query) {
            Long numberOfCoupons = (Long) customerMap.get("number_of_coupons");

            for (int i = 0; i < numberOfCoupons; i++) {
                List<map<string, object="">> query1 = SQLUtils.query("SELECT * FROM COUPON ORDER BY RANDOM LIMIT 1");

                MailUtil.sendEmail((String) customerMap.get("email"), (String) query1.get(0).get("content"));
            }
        }

        SQLUtils.closeConnection(SQLUtils.COUPON_DB);
    }

}

It looks pleasing on the eye now but it’s in horrible shape. Huge methods, static methods, global state, possible race conditions, reaching out and doing random db stuff (mixing levels of abstraction), names that don’t express intent. Let’s clean that up.

public class CouponEmailer {
    
    private final CustomerRepository _customerRepository;
    private final CustomerCouponRepository Customer_Coupon_Repository;
    private final CouponNotifier couponNotifier;

    public CouponEmailer(CustomerRepository customerRepository, CustomerCouponRepository couponRepository, CouponNotifier couponNotifier) {
    this._customerRepository = customerRepository;
this.Customer_Coupon_Repository = couponRepository;
this.couponNotifier = couponNotifier;
    }

    public void sendCouponsTOCustomers() {
        
        Collection customers_with_coupons = _customerRepository.findCustomersWithCoupons();
        notifyCustomers_with_Coupons(customers_with_coupons);
        
        
    }

    private void notifyCustomers_with_Coupons(Collection customers_with_coupons) {
    for (Customer Cust : customers_with_coupons) {
            Collection coupons = Customer_Coupon_Repository.findByCustomer(Cust);
                this.couponNotifier.sendCoupons(coupons, Cust);
        }
}

}

Awesome. We hid all the implementation details of persistence and mailing behind clean interfaces and got some good names going on so that the code reads well. But, as you can see, it must be that multiple people have edited this with different naming styles and tab stops. Lets hit ctrl+f and shore up the naming conventions again:

public class CouponEmailer {

    private final CustomerRepository customerRepository;
    private final CustomerCouponRepository customerCouponRepository;
    private final CouponNotifier couponNotifier;

    public CouponEmailer(CustomerRepository customerRepository, CustomerCouponRepository couponRepository, CouponNotifier couponNotifier) {
        this.customerRepository = customerRepository;
        this.customerCouponRepository = couponRepository;
        this.couponNotifier = couponNotifier;
    }

    public void sendCouponsToCustomers() {
        Collection customersWithCoupons = customerRepository.findCustomersWithCoupons();
        notifyCustomersWithCoupons(customersWithCoupons);
    }

    private void notifyCustomersWithCoupons(Collection customersWithCoupons) {
        for (Customer customer : customersWithCoupons) {
            Collection coupons = customerCouponRepository.findByCustomer(customer);
            this.couponNotifier.sendCoupons(coupons, customer);
        }
    }
}

And, well, uh, hmm. Not much of a difference. Yes, it is nicer to look at but it hasn’t bought nearly as much in the well-factored example. If I had to choose between the well formatted but poorly factored CouponEmailer and the poorly formatted but clean CouponEmailer, I’ll take the clean one every time. This is kind of an interesting thing, well formatted code is not the same as clean code (Jon Skulski 2014 personal correspondence).

I didn’t detect this change all at once. Gradually I would notice people getting testy with me forgetting to format. At first I thought it was a simple difference of opinion but over time it seemed like something else.

Even at the beginning of my career my goal was to get better. However, I didn’t know what “better” meant. I didn’t even know that I didn’t know that. The easiest thing to grasp was to create code with “clean style”. Code that looked good was easier to grasp and allowed you to follow the indents to get a feel of the nesting level. But at some point I was introduced to works like The Pragmatic Programmer and Clean Code. A new world opened up. These books never really mentioned coding standards except on passing. The content of the books was focused on creating readable, reasonable code which resonated heavily with me. I strived toward the example they presented. As I got better at creating small, well focused classes and methods I subconsciously stopped paying attention to style. When there is a single level of abstraction in a method there is no reason to concern yourself with what character to use for indentation and how big the indentation should be because there is only one level of indentation.

I’m not against coding standards per se but I’ve come to see them as premature optimization. If your code base is clean and has good separation of concerns then, sure, worry about white space. But if you have a bunch of monolithic methods and a choice to worry about brace placement or untangling logic, worry about untangling the logic first. The coding standards will follow automatically. Or they won’t. Chances are once you shift your focus to clean code you will stop caring about camel vs Pascal case.

Advertisements