Tag: best practices
Photo by Ales Nesetril on Unsplash
Editorial note: I originally wrote this post for the SubMain blog. You can check out the original here, at their site. While you’re there, download and try their CodeIt.Right product.
One topic in software development that really fascinates me is coding best practices. I’m always searching for ways to improve my work and deliver value in a fast and consistent manner.
It can be tricky to define what a “coding best practice” is. Some people are even in favor of downright retiring the term! But one thing pretty much everyone agrees upon is this: coming up with and implementing strategies—by whatever name you call them—to improve the output of one’s work is something that any developer worth his or her salt should be continuously doing.
Of course, there’s no free lunch. The adoption of a best practice takes time…and sometimes you just don’t have much of that to begin with. And then there’s management, whose buy-in is not always guaranteed.
So, what to do if your development team is struggling with the poor quality of a codebase while lacking time to implement best practices that would help?
The answer I offer you today is what I’ll call the “coding best practices emergency pack”: a small list of coding best practices that you can adopt on relatively short notice to get your team and your codebase from utter chaos to a more manageable state.
Because there’s lots of advice on coding best practices out there, to the point where it’s hard not to feel overwhelmed, I narrowed down my list of emergency-pack best practices by requiring they meet three criteria:
- They must be fundamental, in the sense that they’re the building blocks with which you can implement more sophisticated practices later.
- You can adopt them in relatively short notice. (I’d say a week is feasible.)
- Their cost is free or very low.
The practices that follow all fit these parameters. And without further ado, here it is: my coding best practices emergency pack, with items listed in the order they should be implemented and starting with the most critical one.
Version Control System
I once worked for a software development shop where no version control system was used. The source files were placed in a shared folder that every developer could access. What was the process we used when editing a file? Yeah, you guessed it: we’d simply create a copy of the file and rename it to “filename_old.ext” or something like that.
This was about eight or nine years ago. So maybe things have improved, right? Well, they certainly have, to some extent, but not completely. There are still companies out there that don’t use a VCS.
How to Proceed?
From now on, I’ll just assume you agree that a VCS is a fundamental coding best practice. If that’s not the case, there’s plenty of resources out there explaining what a VCS is and why should you use one.
With that out of the way, it’s time to get to specifics. Which tool should you use? How to go about its adoption?
Git is a solid choice. And despite having a steeper learning curve for those more used to centralized version control systems, such as Subversion or TFVC, it’s de facto standard in our industry. So by all means, learn it, since not doing so can harm your career in the future.
But it’s possible that Git is not the best choice for your team right now. Remember, you’re short on time. So we need to get your team to adopt these coding best practices ASAP.
How do we do this? So, let’s say you have experience with Subversion, having used it at your previous company, but you have no experience with Git at all. If that’s the case, I’d say Subversion is the best choice for you. The overhead of learning a new system and teaching it to your co-workers while putting it into production would be too great.
I’m not going to lie: I love code reviews—and I’m not alone in that. I’ve witnessed firsthand how a good code review can reduce the number of bugs in a codebase, make the code look and feel more consistent, and perhaps best of all, spread knowledge throughout a development team.
And here’s a major selling point: a code review practice is relatively easy to implement. Start as simple as you can, and then tweak and experiment with your approach as the need arises.
What Do I Mean by Code Review?
Talking about “code review” can be tricky. People sometimes mean widely different things when they use the term, so I think it warrants further clarification.
I’m not in favor of a highly stressful and bureaucratic code review process, where your code is scrutinized and criticized in public for hours. I don’t believe in public shaming as a tool for achieving quality. On the contrary, the type of code review I advocate for is a lightweight and low-stress process, usually initiated by submitting a pull request or using your favorite IDE.
How to Proceed
Since we’re now on the same page about what a code review should look like, how would one go about implementing the practice? My answer is, not surprisingly, “the simplest way that could possibly work.”
For instance, if yours is a .NET shop using TFS/TFVC, you can start by installing a check-in policy that requires a code review for each check-in. If your team uses GitHub, you can use pull requests. Just start performing reviews so you and your team can get used to it. Then, with time, start tuning and perfecting your approach.
Here are some of the questions that can appear as you refine your process for this:
- What’s the goal of a code review? Are we looking for bugs? Trying to improve readability? Checking adherence to the company’s coding standard?
- Where do we draw the line between “suggestions” and “impediments”? Is it OK to give a thumbs-down to someone’s code for bad indentation or a slightly off variable name?
- What do when reviewer and reviewee can’t come to a consensus? Bring in a mediator to give the final word? And who should be this mediator? The lead developer?
The answer to all of these questions can be found in automation. Much of the awkwardness of a code review can be removed when you employ a code analyzer to handle the automatable portions of the process.
For instance, SubMain’s CodeIt.Right will give you real-time feedback from inside Visual Studio, alerting you of possible coding issues and even automatically fix code smells and violations for you.
By employing automation, you set your developers free to worry about higher level concerns when performing reviews, such as code clarity or architectural decisions.
You may be thinking that I’ve got it wrong. After all, does it even make sense to talk about automated builds without mentioning automated tests?
Well, I’m going argue that yes, it does make sense, and for one very simple reason: it eliminates “it works on my machine” syndrome.
By having a central place where builds are performed, you shed light on all kinds of problems, from poor management of dependencies to bad test discipline.
How to Proceed
My advice here is the same as before: do the simplest thing that could work.
If your team already uses TFS, then learn how to create a build definition and you’re good to go. On the other hand, if you host your projects on GitHub, you might be interested in taking a look at Travis CI.
With time, you should improve your strategy. Remember the static code analyzers I mentioned earlier? You can integrate them into your build process. Unit testing and other kinds of automated tests are a very important addition as well.
Speaking of which…
You might be surprised to see that I haven’t included unit testing in the list of coding best practices, despite being myself a firm believer in the importance of automated testing to the overall quality of a codebase. And why is that?
Adding unit tests to a legacy application, unfortunately, is hard, to the point that there’s even a famous bookthat focuses solely on this. It’s just not a feasible task for you to tackle quickly.
In a similar fashion, it’s possible that a portion of readers expected me to talk about clean code or the SOLIDprinciples. I do encourage you to research and learn about these topics, but I don’t think they’re a good fit for the purpose of this post. They are, as the name already points out, principles. Think of them as philosophical guidelines—useful, but not as easy to break down into simple, actionable advice.
Deploy Your Package ASAP!
It’s possible that some of you found these practices to be extremely basic and not post-worthy. “Who doesn’t use version control in twenty-freaking-eighteen???” I hear you saying.
Well, it really doesn’t take long to find evidence (anecdotal, but still) that things are not all sunshine and rainbows. To believe that even basic coding best practices, such as using version control or automated testing, are universally applied is probably more wishful thinking than what we’d like to believe.
For the rest of you, I hope this list proves useful.
You know what they say. “When in a hole, stop digging.” And that’s exactly the type of help I wanted to offer with this post: a quick and easy fix, meant to give you and your teammates just enough sanity that you can focus and regain control of your application, ensuring its long-term health.read more...
Editorial note: I originally wrote this post for the SubMain blog. You can check out the original here, at their site. While you’re there, have a look at CodeIt.Right, which can help you with time-related issues and much more.
Do you remember the “falsehoods programmers believe about X” meme that became popular among software blogs a few years ago? The first one was about names, but several others soon followed, covering topics such as addresses, geography, and online shopping.
My favorite was the one about time. I hadn’t thought deeply about time and its intricacies up until that point, and I was intrigued by how a fundamental domain could be such a fertile ground for misunderstandings.
Now even though I like the post, I have a problem with it: it lists wrong assumptions, and then it basically stops there. The reader is likely to leave the article wondering:
- Why are these assumptions falsehoods?
- How likely is it that I’ll get in trouble due to one of these assumptions?
- What’s the proper way of dealing with these issues?
The article is interesting food for thought, but I think it’d make sense to provide more actionable information.
That’s what today’s post is about. I’m going to show you four common mistakes C#/.NET developers make when dealing with time. And that’s not all. I’ll also show what you should do to avoid them and make your code safer and easier to reason about.
1. Naively Calculating Durations
Consider the code below:
Will this code work? It depends on where and when it’s going to run.
When you use
DateTime you get represents the current date and time local to your machine (i.e., it has the
Kind property set to
If you live in an area that observes DST (Daylight Saving Time), you know there’s one day in the year when all clocks must be moved forward a certain amount of time (generally one hour, but there are places that adjust by other offsets). Of course, there’s also the day when the opposite happens.
Now picture this: today is March 12th, 2017, and you live in New York City. You start using the program above. The
StartMatch() method runs at exactly 01:00 AM. One hour and 15 minutes later, the
EndMatch() method runs. The calculation is performed, and the following text is shown:
Duration of the match: 00:02:15
I bet you’ve correctly guessed what just happened here: when clocks were about to hit 2 AM, DST just kicked in and moved them straight to 3 AM. Then
EndMatch got back the current time, effectively adding a whole hour to the calculation. If the same had happened at the end of DST, the result would’ve been just 15 minutes!
Sure, the code above is just a toy example, but what if it were a payroll application? Would you like to pay an employee the wrong amount?
What to Do?
When calculating the duration of human activities, use UTC for the start and end dates. That way, you’ll be able to** unambiguously point to an instant in time**. Instead of using the
Now property on
DateTime, use `UtcNow to retrieve the date time already in UTC to perform the calculations:
What if the
DateTime objects you already have are set to Local? In that case, you should use the ToUniversalTime() method to convert them to UTC:
A Little Warning About ToUniversalTime()
The usage of
ToUniversalTime() — and its sibling,
ToLocalTime()— can be a little tricky. The problem is that these methods make assumptions about what you want based on the value of the Kind property of your date, and that can cause unexpected results.
ToUniversalTime(), one of the following things will happen:
Kindis set to UTC, then the same value is returned.
- On the other hand, if it’s set to Local, the corresponding value in UTC is returned.
- Finally, if Kind is set to Unspecified,** then it’s assumed the datetime is meant to be local, **and the corresponding UTC datetime is returned.
The problem we have here is that local times don’t roundtrip. They’re local as long as they don’t leave the context of your machine. If you save a local datetime to a database and then retrieve it back, the information that’s supposed to be local is lost: now it’s unspecified.
So, the following scenario can happen:
- You retrieve the current date and time using
- You save it to the database.
- Another part of the code retrieves this value and, unaware that it’s supposed to already be in UTC, calls
- Since the datetime is unspecified, the method will treat it as Local and perform an unnecessary conversion, generating a wrong value.
How do you prevent this? It’s a recommended practice to use UTC to record the time when an event happened. My suggestion here is to follow this advice and also to make it explicit that you’re doing so. Append the “UTC” suffix to every database column and class property that holds a UTC datetime. Instead of Created, change it to CreatedUTC and so on. It’s not as pretty, but it’s definitely more clear.
2. Not Using UTC When It Should Be Used (and Vice Versa)
We could define this as a universal rule: use UTC to record the time when events happened. When logging, auditing, and recording all types of timestamps in your application, UTC is the way to go.
So, use UTC everywhere! …Right? Nope, not so fast.
Let’s say you need to be able to reconstruct the local datetime — to the user’s perspective — of when something happened, and the only information you have is a timestamp in UTC. That’s a piece of bad luck.
In cases like this, it’d make more sense to either (a) store the datetime in UTC along with the user’s time zone or (b) use the
DateTimeOffset type, which will record the local date along with the UTC offset, enabling you to reconstruct the UTC date from it when you need it.
Another common use case where UTC is not the right solution is scheduling future local events. You wouldn’t want to wake up one hour later or earlier in the days of DST transitions, right? That’s exactly what would happen if you’d set your alarm clock by UTC.
3. Not Validating User Input
Let’s say you’ve created a simple Windows desktop app that lets users set reminders for themselves. The user enters the date and time at which they want to receive the reminder, clicks a button, and that’s it.
Everything seems to be working fine until a user from Brazil emails you, complaining the reminder she set for October 15th at 12:15 AM didn’t work. What happened?
DST Strikes Back
The villain here is good old Daylight Saving Time again. In 2017, DST in Brazil started at midnight on October 15th. (Remember that Brazil is in the southern hemisphere.) So, the date-time combination the user supplied simply didn’t exist in her time zone!
Of course, the opposite problem is also possible. When DST ends and clocks turn backward by one hour, this generates ambiguous times.
What Is the Remedy?
How do you deal with those issues as a C# developer? The
TimeZoneInfo class has got you covered. It not only represents a time zone but it also provides methods to check for a datetime validity:
What should you do then? What should replace the “do something” comments in the snippets above?
You could show the user a message saying the input date is invalid. Or you could preemptively choose another date for the user.
Let’s talk about invalid times first. Your options: move forward or backward. It’s somewhat of an arbitrary decision, so which one should you pick? For instance, the Google Calendar app on Android chooses the former. And it makes sense when you think about it. That’s exactly what your clocks already did due to DST. Why shouldn’t you do the same?
And what about ambiguous times? You also have two options: choose between the first and second occurrences. Then again, it’s somewhat arbitrary, but my advice is to pick the first one. Since you have to choose one, why not make things simpler?
4. Mistaking an Offset for a Time Zone
Consider the following timestamp: 1995-07-14T13:05:00.0000000-03:00. When asked what the -03:00 at the end is called, many developers answer, “a time zone.”
Here’s the thing. They probably correctly assume that the number represents the offset from UTC. Also, they’d probably see that you can get the corresponding time in UTC from the offset. (Many developers fail to understand that in a string like this, the offset is already applied: to get the UTC time, you should invert the offset sign. Only then should you add it to the time.)
The mistake is in thinking that the offset is all there is to a time zone. It’s not. A time zone is a geographical area, and it consists of many pieces of information, such as:
- One or more offsets. (DST is a thing, after all.)
- The dates when DST transitions happen. (These can and do change whenever governments feel like it.)
- The amount of time applied when transitions happened. (It’s not one hour everywhere.)
- The historical records of changes to the above rules.
In short: don’t try to guess a time zone by the offset. You’ll be wrong most of the time.
It’s About Time…You Learn About Time!
This list is by no means exhaustive. I only wanted to give you a quick start in the fascinating and somewhat bizarre world of datetime issues. There are plenty of valuable resources out there for you to learn from, such as the time zone tag on Stack Overflow or blogs such as Jon Skeet’s and Matt Johnson’s, who are authors of the popular NodaTime library.
And of course, always use the tools at your disposal. For instance, SubMain’s CodeIt.Right has a rule to force you to specify a
IFormatProvider in situations where it’s optional, which can save you from nasty bugs when parsing dates.
Editorial note: I originally wrote this post for the SubMain blog. You can check out the original here, at their site. While you’re there, have a look at CodeIt.Right, which can help you with automated code reviews.
Some weeks ago, I was browsing Twitter when I saw this:
Pair Programming > Code Review— Rafael Ponte (@rponte) November 19, 2017
This prompted a brief discussion between the author and me. He made good arguments, but I left unconvinced that pair programming was the obvious winner.
As someone who’s implemented code review with success and also paired to some extent, I could see how both practices can be valuable. But is one of them clearly better than the other? Are code review and pair programming interchangeable, or are there scenarios in which one clearly shines?
That’s what I’m going to answer today. Let’s dive in.
What I Mean By Code Review
Picture this: you’re a young programmer on your first development job. After you finish your first assignment, they call you to a meeting room; there you find your tech lead and three senior developers. There’s also a large monitor displaying your code. Time for review!
For two-and-a-half excruciating hours, they scrutinize your work while you sweat profusely. From criticizing your high-level design decisions to nitpicking over the most trivial stylistic preferences, nothing gets a free pass.
Is this the scene your mind conjures when you see the words “code review”?
Good news, then. That’s not the kind of review I’m talking about. Instead, think of a very informal and lightweight process. You submit your code for review by creating a pull request or maybe using some embedded functionality on your IDE.
After touching briefly on the what and how of a code review, let’s get to the why. Why should your team adopt this practice? What are the benefits?
The first reason is, not surprisingly, to catch bugs. I’d bet you’re familiar with that old piece of software wisdom that says that the later a defect is found, the higher the cost of fixing it. Then why not use a relatively cheap process that can catch up to 60% of defects?
Here’s another reason your company should perform code reviews: to improve the readability of the code. Trying to read and understand some new piece of code frequently leads to the discovery of issues such as
- Poor naming.
- Bad indentation.
- Instances of cargo cult programming.
- Non-idiomatic code.
The reviewer could also spot overlooked corner cases or help the author assess how performative their code is.
And here’s one last reason. A well-done code review can spread knowledge throughout the team. This improves the product’s quality by destroying knowledge silos.
For the most part, their complaint is time. When you submit some code for review, you’ll have to wait until the reviewer is done.
What should you do while waiting? Ideally, your team should break up the available work in small, discrete units so it can work on these pieces somewhat independently. That’s not always feasible, though. And by the way, the constant switching of tasks might be detrimental to the developer’s focus and productivity.
Let’s say Bob spends x hours implementing a feature. Then Alice reviews his work and says his implementation is completely wrong, and he needs to start from scratch. Those x development hours were just thrown away.
Finally, people sometimes will waste ridiculous amounts of time arguing on pointless stylistic details, such as the position of braces or whether or not to include an underscore before a private field’s name. These debates can unfortunately escalate to levels that turn the workplace toxic.
Pair Programming: Not Just a Super Code Review
Pair programming is a technique in which two developers collaborate on the code together, sitting at one workstation.
They periodically take turns in two roles. The driver writes the code, thinking out loud in order to explain their design decisions and thought process. The navigator observes the driver’s work, giving real-time feedback and advice.
So, is it just “code review on steroids”, as put by Jeff Atwood?
Maybe not. One of the basic principles of agile methodologies is the shorter your feedback loop, the better you are. You can see how getting someone to review your code sooner rather than later works in harmony with the principles that make agile so successful.
Well, it should be no surprise that many benefits of pair programming are also benefits of code reviews, such as fewer bugs, improvement in code readability, and knowledge dispersion throughout the team.
Pair programming may provide exclusive benefits as well, such as:
- Higher focus. The presence of a peer may exert some pressure to stay motivated to solve the task at hand.
- Shorter feedback cycle. Since your pair is reviewing your code in real time, there’s a lot less risk of wasting time due to delayed feedback.
- Increase in usage of other good engineering practices. As suggested in a study done at North Carolina State University, teams using pair programming tend to increasingly use development practices such as unit testing, continuous integration, and establishing coding standards.
As in the case of the code review, pair programming is far from being a universally accepted practice. While many developers love it, others don’t have such happy stories to tell. So, what are some of the most cited problems with pairing?
Let’s start with one common complaint: pair programming can be exhausting. In fact, many claim that pairing is more effective when used for shorter blocks of time—from one and a half to two and a half hours.
Obviously, an odd number of team members doesn’t work well for pair programming. But a shifting number of available personnel is inevitable.
Next on our list of problems is the fact that paring isn’t very remote friendly. You can better imagine the issue after hearing what Daniel Kaplan, who wrote “What It’s Like to Pair for a Year,” had to say about pairing:
These scheduling interruptions happen, but on a typical day we avoid them by having the pairs show up at the same time (for breakfast and standup), go to lunch at the same time, and leave at the same time. This maximizes the time the pairs are pairing.
So, pairing requires synchronicity, which might make it a non-option for remote teams (or even co-located teams with flexible hours).
Some people argue that pairing can undermine creativity and prevent experimentation. While pairing, it’d be rude to waste your pair’s time trying some experimental approach that might ultimately end up not working. So, the safest possible design tends to always prevail, even if it’s not the best possible one.
Finally, pairing doesn’t really provide one of the key benefits of the after-the-fact code review: a person with zero context analyzing the code. The two developers share a context from the beginning of the session, and the effect this has cannot be underestimated. People tend to overvalue their contributions and get emotionally attached to what they create; that’s why it’s so important to get a second person that doesn’t have this attachment and is thus able to provide a clearer judgment.
Code Review vs Pair Programming: The Verdict?
I’ve reached the conclusion that, although code review and pair programming seem equivalent, they’re really not. There is some overlap, but each practice also presents some unique benefits and challenges.
There’s no getting around that pair programming, despite its benefits, requires an even number of people, working at the same time. If your team consists of developers living across several time zones (or even a co-located team with extremely flexible hours), then it’s a no-brainer: code review to the rescue.
If your team doesn’t fit the above description, then give pair programming a try. As long as you work hard to accommodate and have empathy for different kinds of personalities and don’t make it mandatory, it can be beneficial to your team.
Finally, there’s nothing stopping you from using both. You could adopt pair programming as the default MO and leave soloing and code review to fill in the gaps where pair programming doesn’t quite fit so well.
No Matter What You Do, Embrace Automation
Imagine you write for a publication, such as a magazine. After you have a draft, you submit it for review. Would it make sense for the editor to spend all their time looking for spelling errors? Of course not! We have automated spell-check for that, freeing the editor to look for more high-level problems, such as poorly chosen words, lack of cohesion, inappropriate tone, and all those things your English teacher kept nagging you about in high school.
With software, things should not be so different. By using an automated code review tool, you can eliminate a lot of the bickering that often occurs in code reviews or pair programming sessions. There will be no arguing about naming and formatting conventions, the position of brackets, and others pointless trivia.
You can also employ a static analysis tool to warn you about potential bugs and opportunities for refactoring. That way, the reviewer/navigator is free to focus on the high-level stuff that requires human creativity, intelligence, and empathy.
Trust and Respect
While researching for this post, one theme kept reappearing: that code review emerges from a lack of trust in our developers or that pair programming infantilizes them.
I couldn’t disagree more.
Precisely because we respect our developers—and our clients—we should employ techniques and tools at our disposal to improve the quality of our work.
It’s not about lack of trust. It’s about recognizing that programming is hard and sometimes, just one brain isn’t up to the task.read more...
Editorial note: I originally wrote this post for the NDepend blog. You can check out the original here, at their site. While you’re there, download NDepend and give it a try.
I first learned about cargo cult programming a few years ago. I remember thinking back then, “What a strange name for a programming-related concept.”
If you share my past self’s astonishment, then today’s post is for you!
First, you’ll see what cargo cult programming is and why you should care. Then, we’re going to look at some practical examples, using the C# language. Finally, we’ll close with advice about what you can do, as a developer, to avoid falling into this trap.
Cargo Cult Programming: Doing Stuff Just Because
According to Wikipedia, “Cargo cult programming is a style of computer programming characterized by the ritual inclusion of code or program structures that serve no real purpose.”
In other words, it’s when a developer writes code without really understanding it. The developer may use a very trial-and-error approach—maybe copy and paste some code from somewhere else and then tweak it and test it until works, or sort of works. Then the developer will stop tweaking the code, for fear it will stop working. In the process, maybe they leave some lines of code that don’t do anything.
Or maybe they tried to use an idiom they picked up from another developer while failing to understand that the contexts are different and it’s useless in the current situation.
Finally, it might just be lack of education: maybe the developer has a poor mental model of how the tools they’re using really work.
Why is Cargo Cult Programming a Problem?
As Eric Lippert puts it, cargo cult programmers struggle to make meaningful changes to a program and end up using a trial-and-error approach since they don’t understand the inner workings of the code they’re about to change.
This is not so different from what the Pragmatic Bookshelf calls “programming by coincidence”:
Fred doesn’t know why the code is failing because he didn’t know why it worked in the first place. It seemed to work, given the limited “testing” that Fred did, but that was just a coincidence.
That single sentence pretty much sums it up for me: if you don’t know how or why your code works, neither will you understand what happened when it no longer works.
Origin of the Term
Although practices that are considered cargo cult today have been recorded as early as the late 19th century, the term itself dates from 1945, when it was first used to describe practices that emerged during and after World War II between Melanesian islanders.
These islanders would mimic the soldiers’ behavior, such as dressing up as flight controllers and waving sticks, hoping that airplanes would descend from the skies with a lot of cargo.
Since then, the term cargo cult has been used in a variety of contexts to mean to imitate form without content—to perfectly copy the superficial elements while failing to understand the deeper meanings and workings of whatever one’s trying to emulate.
Talk is Cheap; Show Me the Code!
Enough with the history lesson. Time to see some code! I’m going to show you five examples of cargo cult programming in the C# language.
Checking a Non-Nullable Value Type for Null
This one is a pet peeve of mine since I see it a lot in production code. It goes like this:
Here we have a developer that probably doesn’t grok the difference between value and reference types. It would be completely forgivable, in the case of a junior developer, except for the fact that the compiler warns you about that.
You could argue that I’m nitpicking. After all, the code will run fine in spite of this. In fact, the check won’t even be included in the resulting IL, as you can see from this print of a decompiling tool:
You can see in this code snippet that the compiler has optimized the null check out.
There are plenty of worse problems, granted. Yes, the application won’t crash because of this. So what’s the big deal?
Well, for starters, I’d be worried about a development shop where the sole quality measure was “it runs without crashing.” But the real problem is that this type of code shows a lack of understanding of some fundamental characteristics of the language and platform that can bite you in the future.
Unnecessary Use of ToList() in LINQ to Object Queries
Like the previous one, this is something I routinely see in production code. Consider the code below:
The problem we have here is that these calls to
ToList() are completely unnecessary (except maybe the last one, if you really needed the result to be a
List and not only an
In my experience, this happens when the developer doesn’t understand the nature of LINQ; they erroneously think that the LINQ methods belong to the concrete type
List<T> instead of being extension methods that can be used with any
ToList() several times like this, the developer creates several new lists, which can be detrimental to the performance of the application.
You could rewrite the code above like this:
Consider the following line:
Here we have not only one but two unnecessary conversions. First, the developer creates a new string and then parses it to
DateTime when a simple cast would have sufficed:
This example assumes that the underlying database type is some specific type for dealing with dates (for instance,
datetime in SQL Server). Of course, if you were using an inadequate type (such as
varchar) then this would be a problem of its own.
Also known as Pokémon syndrome (“Gotta catch ’em all!”), the anti-pattern here is to add a try-catch block to every single line that could possibly throw an exception.
Bonus points if the code is attempting to catch
System.Exception instead of a more specific exception, thus blurring the distinction between expected and unexpected errors.
More bonus points if the catch block doesn’t contain any code at all!
The general advice here is this: never catch unless you have a very specific reason for doing so. Otherwise, just the let the exception bubble up until it’s dealt with by the top-level exception handler.
If this advice seems vague (“How would I know if I have the right reason for catching an exception?”), that’s because it is vague. It’s beyond the scope of this post to go deeper into this matter, but Eric Lippert’s excellent article called “Vexing Exceptions” will greatly improve your understanding of exception handling.
Using StringBuilder Everywhere
It’s the stuff of superhero movies: after reading somewhere that concatenating strings by using the ‘+’ operator is incredibly inefficient, the well-meaning developer takes upon themselves the Herculean task of updating every single concatenation in the codebase to
The reasoning for this is, of course, that
System.String is immutable. So every time you “modify” a string, you’re in fact creating a new instance in memory, which can hurt performance pretty badly.
Well, guess what? The compiler is pretty smart. Let’s say you have the following line:
This, in fact, gets translated to
The fast rule of thumb is it’s fine to use the simple concatenation when you know the number of strings to append in compile time. Otherwise, a
StringBuilder probably makes more sense.
Of course, some scenarios aren’t that clear-cut. The only advice worth giving here is to do your homework. When in doubt, research and benchmark to your heart’s content.
I’ll leave you with more sound advice from Eric Lippert:
Unnecessary code changes are expensive and dangerous; don’t make performance-based changes unless you’ve identified a performance problem.
Is There a Remedy?
I’d say it’s fair to assume that more inexperienced developers are more prone to commit mistakes due to cargo cult programming. But no developer is really immune to it, independent of their knowledge or experience.
We’re only human after all. Tiredness, deadlines, cognitive biases, and (to be really honest) the eventual laziness can turn even the best developer into a cargo cult programmer.
Unfortunately, there’s no 100% guaranteed way of preventing this from happening. Yet there are some measures you could take to, at least, decrease the odds.
Let’s take a look at some of them.
Use Code Review/Pair Programming
The first measure you could take to avoid cargo cult programming is to simply get a second pair of eyes on your code. The benefits of having a second person reviewing each line of code before it goes to production can’t be overstated. And while code review and pair programming aren’t perfect equivalents, both of these practices will bring you this benefit.
Always Test Your Hypothesis
Write unit tests (and other types of tests as well). Monitor your application in production. If something doesn’t perform well, benchmark the heck out of it. Don’t just assume things. Testing your hypothesis can bring valuable insights and save you when your intuition gets it wrong.
Read Other People’s Code
Reading other people’s code is a great way to learn. It’s a perfect tool to compare your own ideas and assumptions against what other developers are doing, exposing you to novel concepts that can force you to gain a deeper understanding of the issues at hand.
In the era of GitHub, there isn’t much of an excuse for not doing that.
Learn From Your Tools
There are currently a plethora of tools that can help your team improve the quality of their code. Here’s the thing, though: you shouldn’t just use these tools. You should also learn from them. If you use NDepend, read about its rules. Try and understand the rationale behind them. What are the principles and best practices that guided its authors when coming up with the rules?
The same goes for other types of tools—and even the warnings the compiler gives you.
Computer Science, Not Computer Superstition
Even though no one is immune to cargo cult programming, we should strive to overcome it. There’s hard-earned industry wisdom at our disposal, slowly generated over more than seven decades. Let’s use it. Let’s understand our tools and our craft and write better software.read more...
Spoiler Alert: You should avoid most of them.read more...
Some people believe private methods should be avoided. Are they right?read more...
There are only two hard things in Computer Science: cache invalidation and naming things.
Do you want to write great code? Clean, understandable, human-readable code? Well, there are several skills you need to acquire. But I’d say #1 on the list is “Picking Good Names”.read more...