How We Deal With Ugly Code

How We Deal With Ugly Code

As consultants who are active on social media, people sometimes reach out asking for our opinion. And we’re happy to offer it up, even if it isn’t anything different that what you’d hear from any senior developer. Recently we were asked by someone whether we are bothered by badly formatted code, and what do we do about it.

xkcd: code quality

Answer: Yes, we see badly formatted code constantly, and it does bother us, as well formatted code makes life easier on the next guy who looks at it – be that a future you, or someone else.

And yes, we’ll try to clean it up, but several things need to be true first:

  1. There has to be agreement on what the right way is to reformat the code. We can’t merely be imposing our personal opinion. That means either using a client’s style guide, if they have one, or developing one for them and getting their developers involved in the process.
  2. If you’re working on a small portion of a large code base that widely violates the desired coding standards, you really need buy-in from the team leader or engineering manager. They have to be committed to migrating to the preferred format, otherwise you’re creating an “island” of pretty code that will just be an oddity.
  3. Ideally, you want there to be automated tests in place. Preferably unit tests, but at least system tests. Because even benign reformatting can introduce unintentional errors.

It’s not that often that all of these things are true. Most teams lack a style guide. Most managers are focused on things other than code clarity, and aren’t committed to cleaning up the whole codebase. Most projects lack tests.

So what is more typical is that surgical edits are made to modules while adhering to the style already present in the file.

But when the requested code changes are large enough in scope, we’ll definitely lobby for updating the style, if the code needs it.

 

A follow-up question: if you do clean up the code, how do you avoid getting complaints from the team over all the noise it causes in the version control diffs?

Answer: At minimum the cleanup work should be done on its own commit with a clearly descriptive commit comment. Always resist the urge to do some reformatting – even if just a few lines – in combination with functional changes. If you do, you’ve just made the job of your code reviewer way harder.

Larger cleanup work across multiple modules should be done on their own branch.

Yeah, it creates noisy diffs, but it’s contained, and people know why it happened.

If you plan to do a large-scale reformatting, you might have some timing considerations if you have multiple developers actively working on branches in parallel, as cleanup can trigger merge conflicts.

So you might want to plan to do a cleanup commit right after a release and before developers fork off branches for the next sprint of work. If you have a style guide worked out in advance, and a Perl::Tidy configuration tuned to implement it, and working unit tests, you might be able to do the cleanup in a matter of hours.

But the approach we typically take is that reformatting is done incrementally as modules get touched for other reasons. The work starts with a reformatting commit.

 

The correspondent wondered if Perl’s expressivity leads to inherently “difficult to read code.”

Answer: In our opinion Perl is no messier than C, or many other languages that don’t impose whitespace constraints.

Adhering to a style is part of being a professional. The AP didn’t invent a new language to get their reporters to write consistent prose, they just supplied them with a style guide. Sure, that means style isn’t universally consistent, but it also means it can be tuned to the organization and can be allowed to evolve over time as new ideas are absorbed from other communities. Preferred Perl style has certainly evolved lots over the last several decades. And generally for the better.

 

Finally the correspondent wondered how to train new developers on writing clean code.

Answer: If code compiles, passes the tests, and does what the customers want, it can be hard to internalize the value of it also looking consistent. That’s a lesson you learn after being tasked with work on legacy code countless times, and having to adjust your reading for a style that might change from function to function.

If you’re in a position to establish a code style for a team, it’s highly recommended that you take the time to negotiate the style rules with the team. It’s guaranteed that there will be differences of opinion, but the process will at least allow those disagreeing to understand the motivation for the style choices, so they don’t feel arbitrary. You want to arrive at buy-in rather than just imposing a standard.

Through this process you should also be educating them on the value of having consistent code. Once there is mostly agreement on a standard, you can then enforce it through your code reviews, and get beyond them dismissing it as “nit picking.”

Creating a Perl::Critic configuration to validate that your style was adhered to, and incorporating that into your continuous integration process can also help.

 

Maintaining code style is a ubiquitous problem. Other than code just written yesterday and validated by Perl::Critic, every code base suffers from this to some degree. I don’t know if we’ve arrived at a solution, but we have learned techniques to help mitigate the problem.

 

Thanks to our correspondent for reaching out with the questions, and if you have one about Perl, or software development techniques in general, feel free to contact us on social media or otherwise.

Tom Metro is founder and Chief Consultant at The Perl Shop. He has been providing software consulting services since 1991 for companies ranging from startups to large enterprises, like Digital Equipment Corporation and Ticketmaster. Currently specializing in agile software development using open source technologies, such as object oriented Perl, MySQL, and JavaScript on medium to large scale web-based applications. As Director of Software Development at WebEvent, Inc., Tom managed a team of 6 developers and several consultants working on a family of 4 commercially released software products through 7 major release cycles and numerous maintenance cycles. Tom also is involved in numerous open source projects, such as MySQL, Firefox, MythTV, Debian, Ubuntu, numerous Perl libraries, and volunteers for local open source related organizations, such as Boston Linux/UNIX user group, Boston Perl Mongers, and North East Perl Workshop.

Leave a Reply

Your email address will not be published. Required fields are marked *

This site uses Akismet to reduce spam. Learn how your comment data is processed.