http://www.perlmonks.org?node_id=1114424

You've got your typical company started by ex-software salesmen, where everything is Sales Sales Sales and we all exist to drive more sales.

On the other extreme you have typical software companies built by ex-programmers. These companies are harder to find because in most circumstances they keep quietly to themselves, polishing code in a garret somewhere, which nobody ever finds, and so they fade quietly into oblivion right after the Great Ruby Rewrite, their earth-changing refactoring-code code somehow unappreciated by The People.

-- The Developer Abstraction Layer by Joel Spolsky

Though my natural inclination is to be a bit OCD about keeping code clean, I concede that spending too much time and money on refactoring, writing programmer tools, and endlessly polishing code will likely lead to commercial failure. As will the converse, namely neglecting your developers and their code and architectures in favour of sales and marketing. Successful software companies tend to have a healthy balance.

Refactoring

Booking.com, perhaps the most commercially successful Perl-based company, has caused a bit of controversy over the years with their attitude towards refactoring. To give you a flavour, I present a couple of comments below:

Booking is destroying my career because I am not allowed to do anything new. I am not allowed to use new technologies. I'm not allowed to "design" anything big. I am not allowed to write tests. I am allowed to copy that 500 line subroutine into another module. If people have done that several times before, maybe it should be refactored instead of duplicated? If you do that, you get in trouble. As one boss says, "we do not pay you to write nice code. We pay you to get job done."

Management, and the term is quite lose when applied to Booking.com, sees no gain in refactoring code. By refactoring I'm talking about taking a few weeks to rewrite an existing piece of software. By definition refactoring doesn't bring new functionality so this is why management is reluctant to go down that road. We're quite lenient about code that gets added to the repo, as long as there's a business reason behind it. If a quick hack can be deployed live and increase conversion then it will be accepted. But rest assured that crappy code doesn't last long, specially if other devs have to use it or maintain it.

-- from Truth about Booking.com (Blog)

One of the posts specifically deals with the culture of "get it done and fast" and how they do not encourage refactoring or basic testing. I actually work in a Perl shop where management has the same kind of mentality, and it is slowly killing our efficiency.

Regarding testing, it's true that we're not very unit testing focused. This is mainly because we've decided to spend most of the time/money/infrastructure that you might usually spend on unit testing on monitoring instead. If you have unit tests you still need monitoring, but in practice if your monitoring is good enough and you have an infrastructure to quickly rollout & rollback systems you can replace much of unit testing with monitoring.

We're not adverse to refactoring when appropriate. But if you're going to propose rewriting some code here you'll actually have to make a compelling case for it which isn't just "the old code is hairy". Do you actually understand what it does? Maybe it's hairy and complex because it's solving a hairy and complex problem. Are you not aware of where this system fits into the big picture? We've also had code that's looks fantastic, had tests, used lots of best practices that we've had to throw away completely because it was implementing some idea that turned out to be plain stupid.

-- from What exactly is up with Booking.com? (reddit)

Opportunistic Refactoring and The Boy Scout Rule

Some people object to such refactoring as taking time away from working on a valuable feature. But the whole point of refactoring is that it makes the code base easier to work with, thus allowing the team to add value more quickly. If you don't spend time on taking your opportunities to refactor, then the code base gradually degrades and you're faced with slower progress and difficult conversations with sponsors about refactoring iterations.

There is a genuine danger of going down a rabbit hole here, as you fix one thing you spot another, and another, and before long you're deep in yak hair. Skillful opportunistic refactoring requires good judgement, where you decide when to call it a day. You want to leave the code better than you found it, but it can also wait for another visit to make it the way you'd really like to see it. If you always make things a little better, then repeated applications will make a big impact that's focused on the areas that are frequently visited - which are exactly the areas where clean code is most valuable.

-- Opportunistic Refactoring (Martin Fowler)

The Boy Scouts have a rule: "Always leave the campground cleaner than you found it"

What if we followed a similar rule in our code: "Always check a module in cleaner than when you checked it out"

-- The Boy Scout Rule (O'Reilly)

At work, we perform opportunistic refactoring following the Boy Scout rule, trusting the judgement of developers. How do you do it at your workplace?

Code Reviews

By way of background, my company went agile about five years ago, at first with great zealotry, nowadays with more maturity and less dogma.

Before check-in, all code must be reviewed, either continuously via pair programming, or via a lightweight code review (typically over-the-shoulder). We also have a coding standard, though it is not strongly enforced.

To give a concrete example, during a code review the other day, I persuaded the author to eliminate unnecessary repetition by changing this snippet:

my $config = <<'GROK'; ADD UDP_LISTENER ( 515 ) ADD UDP_LISTENER ( 616, 657 ) ADD UDP_LISTENER ( 987 ) GROK my @test_cases = ( { desc => "# Test 1", conf => $config, find => [ 'port = 515', 'port = 616', 'port = 657', 'port = 987' + ], }, );
to:
my $liststr = 'ADD UDP_LISTENER'; my @ports = ( 515, 616, 657, 987 ); my $config = <<"GROK"; $liststr ( $ports[0] ) $liststr ( $ports[1], $ports[2] ) $liststr ( $ports[3] ) GROK my @test_cases = ( { desc => "# Test 1", conf => $config, find => [ map { "port = $_" } @ports ], }, );

What would you have done?

I'm sure some other programmers at my company wouldn't have bothered suggesting any changes at all: after all, the code worked as is, it's pretty clear, plus "it's only a test script", so why bother?

Though I felt the code was more maintainable with duplication eliminated, I had another motivation in this specific case: training. You see, the programmer in question was very new to Perl and, as I found out during the review, had never used map before! Training (and improved teamwork) are important benefits of code reviews.

Eliminating unnecessary duplication and repetition is a common discussion topic during code review in my experience. (Note: I did not include this example to argue further about what DRY means exactly in Room 12A :). Other common discussion points during code review are:

Note that we do not normally discuss code layout because all code is pushed through Perl::Tidy before review.

I'm interested to learn about your workplace experiences. In particular:

Cleverness

To finish, here's another one, derived from Clever vs. Readable.

Would this statement pass your code review?

my $value = [ $x => $y ] -> [ $y <= $x ];
If not, would you suggest changing it to:
my $value = $x < $y ? $x : $y;
or:
use List::Util qw(min); my $value = min( $x, $y );
Or something else?

References

Updated Aug 2015: Added Ternary vs. Sort vs. Max reference.