9th most common mistake - using "always" and "never"

The article I've found today in The Morning Brew is the great occasion to break the silence here on my blog. When I read an article that sounds like a definitive source of truth, I don't expect to read so subjective, single point of view arguments as I've found in Paweł Bejger's post "8 Most common mistakes C# developers make". I just couldn't resist to comment on it.

Paweł mixed some .NET Framework tips with performance advices and some architectural guidelines together. Let's briefly discuss his points.

1. String concatenation instead of StringBuilder

Advice here is to use StringBuilder as it doesn't allocate memory for each string part. But it adds its own overhead, it's often an overkill and actually may be a performance issue itself when concatenating few strings only. If StringBuilder was the only proper way to concatenate strings, the framework wouldn't expose such handy + operator for sure.

2. LINQ – ‘Where’ with ‘First’ instead of FirstOrDefault

Two different things were mentioned in this topic. The first one is that instead of chaining Where(x => sth) and First(), we should use First(x => sth) instead. Well, yes, but it's nothing more than one less method call. The number of elements actually fetched from the collection is equal to one in both cases, no performance difference. I would say this is up to personal preferences.

The second is that FirstOrDefault is better than First as it doesn't make an assumption that the collection is not empty. But I often do want to have that assumption. Otherwise, I need to be sure that my code handles null well. In many cases, when I'm sure that the collection is not empty, I just don't need that null handling code. And yes, First will throw for empty collections. I still find it better than some mighty NullReferenceException that may be thrown few lines later in case FirstOrDefault is used without proper attention.

3. Casting by means of ‘(T)’ instead of ‘as (T)’ when possibly not castable

Agreed. But again, not so big difference practically. NullReferenceException thrown unexpectedly later due to unproper handling of null returned by as operator is not a bit better than InvalidCastException thrown directly while casting. Anyway, the need for type casting itself can often indicate a code smell.

4. Not using mapping for rewriting properties

Paweł suggests here that we should use object-to-object mapping libraries like AutoMapper for rewriting properties from one object to another. Agreed, but only when we're doing A LOT of stuff like this, i.e. mapping between layers. When we just need to have several properties mapped, the cost of using the external library will never pay off. Moreover, in those cases it sounds like over-engineering and abuse of both KISS and YAGNI principles. And especially when some additional logic needs to be performed during the mapping, I would personally suggest being as explicit as possible, for readability and maintainability reasons.

5. Incorrect exceptions re-throwing

If we need to re-throw exceptions, we should be using throw alone to preserve the stack trace, agreed. Just a nitpick here - the example used in the article shows more harmful anti-pattern - catching general Exception itself. Wonder how we could ever handle OutOfMemoryException.

6. Not using ‘using’ for objects disposal

Well, can't disagree. Developers should know how to use the tools they were given.

7. Using ‘foreach’ instead of ‘for’ for anything else than collections

Don't disagree here, again, but I haven't really checked that, as for me this really sounds like a micro-optimization not needed in the vast majority of cases. And premature optimization, without known bottlenecks and metrics, even if not the root of all evil, is rather not a time well-spent.

8. Retrieving or saving data to DB in more than 1 call

Similiar to the one above, but on the different level. There's nothing wrong in reading data in two queries if our life becomes much easier then. Modern databases and ORMs offer features like multi-queries or batches (sending multiple commands to the database together to reduce round trips), so the optimization can be moved at infrastructural level, instead of unnecessarily complicating our query.

However, it's almost always wrong to have N+1 queries, but that's the separate story.

Conclusion

I do not negate that C# developers should know all the things mentioned in Paweł's article, but being aware of how things work, what are the pros and cons of each approach is something different than just blindly applying the rules like "don't do this, do that". I'm missing those pieces of advice in the original article.

There's no such thing as the single right answer for any question in our craft. There are always many options to consider and we can never be sure that something is always wrong or always right not knowing the whole context. Even classical design patterns proposed by the Gang of Four, always considered as the best practise we should not discussed with, were recently reasessed quite well by Ayende.

The answer to life, the universe and everything in programming is just always "it depends".