This (partial) screenshot shows some interesting data.  The full screenshot would make things more clear, but where’s the fun in that? Who can tell me what data is being shown here?

A mistake that you commonly see developers make (especially young ones) is to try to achieve perfection when they need to design something.  Whether it’s just a reusable base class, a component, or even a small library or framework, a lot of people get too caught up in details that don’t really matter in most cases.

It is understandable though, and i think most developers have gone down that path quite a few times in their careers.  Especially early on.  You know how it goes… you need to create something that is going to be reused by others, and you want to make sure that it is just perfect.  That nobody can say anything bad about it.  That everyone will agree without discussion that that piece of code is simply great.

So a lot of people in that situation start thinking about things like:

• Shouldn’t i seal this class? Nobody’s ever going to need to derive from this because i’ve left plenty of other extensibility points
• Shouldn’t i seal this method? After all, nobody should ever change the way this particular piece of code works
• This class will never be used by anyone outside of this assembly, so i probably should make it internal, right?
• This particular method will be a bottleneck so i should really make it as fast as i can
• Whenever i can pull some common logic in a base class, or introduce more base classes i should do so!
• Etc…

The reality of the situation is that despite your best intentions, focusing too much on details like that will quite frequently lead to very inflexible code that a lot of people will find hard or annoying to use.  Unless you are very experienced with this kind of stuff (and for the record, i’m not saying that i am) you’re very likely to get these decisions wrong if you think about them up front so it really isn’t worth spending so much time on ‘details’ like that.  In fact, the best thing to do is often to keep things as simple as possible until you actually have a reason to make them more complicated.  Making things more complicated than they need to be in advance never works, and you’re probably going to over-complicate things in places where it turns out not to matter.  If you’re really unlucky, that will actually make it harder to modify or extend other parts that over time really do need to be modified or extended in some ways.

Generally speaking, i think you’re better off focusing on the following goals/principles:

• Avoid writing classes that are slutty
• Make sure that your consumers primarily communicate with your classes through interfaces.  Though you don’t need to put everything behind an interface either… pretty much anything that people might need/want to change in some way typically are good candidates.
• Use Dependency Injection so implementations can easily be switched with others
• Use virtual methods unless you can think of a really good reason not to
• Don’t make your classes internal unless you can think of a good reason to do so (and keep in mind that we can still do whatever we want with them through reflection, and will do so if that turns out to be the best way to get something working the way we need it to if you failed to provide proper extension points)
• Do not seal classes unless you can think of a really good reason to do so
• Do not worry about performance up front, unless for those places where you are going out-of-process (remote services, databases, file systems, etc…)
• Keep your classes small and focused
• Learn about the SOLID principles, and apply them. Keep in mind that going for 100% SOLID code is typically not worth it either.  Go for the 80/20 rule here.

If you keep those things in mind, you will typically end up with code that is flexible to use, and easy to change.

Obviously, all of this assumes that you are in a position where you can go back to the code and make changes.  If you’re releasing frameworks/libraries/components that will be used by a lot of people and can’t afford to break backwards compatibility, then you probably need to be more strict about these things because then you can’t always just go back to change something.  I don’t think it’s a stretch to claim that most developers are not in this situation however, so most of us often don’t need to waste time thinking about those things in advance

Concepts like Dependency Injection and using an Inversion Of Control container have gradually gotten more popular and accepted in the .NET world in the last 2 years or so.  There are however still quite a few people who doubt the validity of these concepts.  Quite a few of these people seem to think that Dependency Injection for instance is only useful to increase the testability of your code. I wanted to go over a real world example which shows a (IMO) huge benefit which can be solely attributed to thinking about loose coupling, using Dependency Injection and using an Inversion Of Control container.

I recently wrote a post about how you can use an Agatha service layer fully in-process instead of having to host it in a different service through WCF.  The only reason why it’s so easy to do that, is because Agatha’s design makes heavy use of loose coupling and dependency injection.  And obviously, it makes use of an Inversion Of Control container to manage the dependencies and to wire everything together.

There are basically 3 very important parts to Agatha.  The first is the Request Processor.  This class basically delegates the handling of each incoming request to its corresponding request handler.  The other important parts are the Request Dispatcher (for synchronous client-side usage) and the Asynchronous Request Dispatcher (for asynchronous client-side usage).  I’ll just refer to these two classes as the request dispatchers for the rest of this post.  When the service layer is hosted through a WCF service, the request dispatchers need to communicate with the Request Processor through a proxy (either a synchronous one, or an asynchronous one).  It would’ve been very easy to tie the implementations of the request dispatchers directly to the implementation of the WCF proxies.  If i had gone that way, i wouldn’t have been able to offer the ability to run the service layer in process though, since the request dispatchers would require the WCF proxies to be used.  I could’ve hosted the WCF service in process, but that would really be overkill if you don’t really want to use WCF.

As easy as it would’ve been to tie the request dispatchers directly to the proxies, it was just as easy to make them depend on a slightly more abstract component.  There are two interfaces to communicate with the Request Processor:

Notice that there are no WCF attributes on these interfaces.  I have two more WCF-enabled interfaces, namely the IWcfRequestProcessor and the IAsyncWcfRequestProcessor.  They define the same methods as their non-WCF counterparts, but have all of the required WCF attributes placed on top of those methods.

Typically when creating (or generating) a WCF proxy, the proxy will only implement the interface of the service contract (in this case, that would be the IWcfRequestProcessor or the IAsyncWcfRequestProcessor interface).  Any class that would want to use these proxies would then need an instance of that proxy to be able to communicate with the WCF service.  In Agatha’s proxies, i chose a slightly different approach.  They implement both the WCF interface as well as the none-WCF interface.  Since the method definitions are identical, this doesn’t require any more work.  Take a look at the class definitions of those proxies:

As you can see, both proxy classes inherit from WCF’s ClientBase class and pass the WCF-specific interface as the generic type parameter to ClientBase.  That means that these proxies can execute the WCF operations defined in the WCF-specific interface.   These proxy classes also implement the original interfaces, which means that these proxies can be used by any class which requires an instance of IRequestProcessor and IAsyncRequestProcessor.

In Agatha, there is no class that directly uses either the RequestProcessorProxy or the AsyncRequestProcessorProxy class.  Instead, the request dispatchers depend solely on IRequestProcessor or IAsyncRequestProcessor:

The dispatchers never know exactly who they’re talking to.  This means that they can communicate either through a WCF proxy, or the real Request Processor, depending on how your Inversion Of Control container is configured.

That is a huge benefit because it gives you a tremendous amount of flexibility when it comes to how you want to use your service layer (in process, in another service, on another machine, whatever) and it really didn’t take any extra effort with regards to development time to achieve that flexibility.  If i want to use Agatha with a different communication technology, then i could do so by simply creating implementations of the IRequestProcessor and IAsyncRequestProcessor interfaces which deal with the specifics of whatever that different communication technology might be.  I would also have to change the way the implementations are registered with the Inversion Of Control container to make sure that the new implementations are used.  But i wouldn’t have to change anything else.

In this case, dependency injection and loose coupling was not used to increase testability.  The increased testability that comes from using this approach is an added bonus.

I’m looking into which Code Metrics we should keep an eye on, or at least pay attention to.  Of all the metrics that NDepend offers, i only seem to find the following ones valuable (some only slightly):

• On the Type level:
  • The number of lines of code of the type
  • Afferent coupling
  • Efferent coupling
  • Cyclomatic complexity
  • Association between classes
  • Number of children
  • Depth of inheritance tree
• On the Assembly level:
  • Efferent coupling
  • Relational cohesion

Some metrics that you’d expect to be interesting but that i’m specifically not interested in:

• On the Type level:
  • Lack of Cohesion Of Methods: i can’t help but think that this metric is calculated incorrectly.  When studying NDepend’s xml output, i noticed quite a few types where the number of instance fields was incorrect (it listed a lot more than there actually were, and it seems to be related to having compiler generated classes when you’re using lambda statements quite heavily).  Since the number of fields metric is used to determine the cohesion of the class, this result just doesn’t seem to be correct if the number of fields is incorrect.
• On the Assembly level:
  • Afferent Coupling: our client-side and server assemblies share no types, except for the types declared in a common assembly.  That common assembly only contains Request/Response types and DTO’s, nothing else.  The afferent coupling of the common assembly is always high, though that metric is meaningless for us.  The afferent coupling of the client-side and server side assembly is 0 since they never use each other directly.  So basically, that metric is useless for us.
  • Instability: Instability is the ratio of efferent coupling to total coupling.  Since total coupling is the sum of efferent coupling and afferent coupling, it is in our case essentially the same as efferent coupling (because our afferent coupling is 0), which means that the ratio of efferent coupling to total coupling is 1, which is supposed to indicate a completely instable assembly.
  • Abstractness: this is the ratio of abstract types (abstract classes and interfaces) to the total number of types.  This is also useless for us since a very high percentage of our classes implements an interface, and usage of those classes always happens through the interface type instead of the concrete type
  • Distance From Main Sequence: this metric is based on instability and abstractness of an assembly… again, useless in our case.

I might be wrong about the whole thing, but i can’t help but think that these metrics are no longer relevant, or even present an incorrect picture about the quality of the code if you’re making heavy use of practices like Dependency Injection and tools like an Inversion Of Control container.

So i’d like to ask you: which metrics do you consider to be important? Which ones do you ignore?  Am i wrong about the ones i’ve listed as not interested in? Are there other metrics that you think i really need to take into account?

I’ve stated previously that reading software development books is a good way of investing in your skills and your career. But which ones should you read? And in what order should they be read? I’ve compiled a list of books that i think can truly increase your skills substantially. I’ve put them in the order in which i believe they will have the most effect, and grouped them in 3 ’stages’. I primarily have young developers who are just getting started as professional developers in mind with this list, but it should be just as useful to developers who’ve been around for a while and simply want to improve.

The first thing that you should focus on is improving your ability to write clean, unambiguous, maintainable code. The following books should greatly help you with that:

1. Test-Driven Development (Kent Beck)
2. Refactoring (Martin Fowler)
3. Implementation Patterns (Kent Beck)
4. Code Complete: 2nd Edition (Steve McConnell)
5. Working Effectively With Legacy Code (Michael Feathers)
6. Clean Code (Robert C. Martin)

The order of this stage might surprise some people, but i’m willing to bet that this is the most efficient order to reading those books.

After you’ve learned how to write great code, you should really start focusing on clean design and architecture. That’s not to say that you should focus solely on design and architecture, but the more you know about it, the better off you will be:

1. Design Patterns (Gang Of Four)
   
2. Patterns Of Enterprise Application Architecture (Martin Fowler)
3. Domain-Driven Design (Eric Evans)
4. Enterprise Integration Patterns (Gregor Hohpe, Bobby Woolf)
5. Release It! Design and deploy production-ready software (Michael T. Nygard)
6. 97 Things Every Software Architect Should Know (edited by Richard Monson-Haefel)

This stage probably deserves a bit of clarification as some of the books listed in this part might be somewhat ‘controversial’. If you read and learned from the books in the first stage, then you should be capable of putting everything you read in the second stage in perspective. You will have learned that you shouldn’t just apply as many patterns as possible, but it’s certainly a good thing to know about their existence.

Finally, you need to learn about working in a team environment and understanding team dynamics. The following books aren’t about working in teams specifically, but contain a tremendous amount of wisdom and insight that will definitely help you when it comes to working in a professional team environment:

1. Extreme Programming Explained, Second Edition (Kent Beck)
2. The Art Of Agile Development (James Shore & Shane Warden)
3. The Mythical Man-Month, 20th Anniversary Edition (Frederick P. Brooks)

This one probably needs a bit of clarification as well. I’m not saying that you should do Extreme Programming. But it certainly won’t hurt you to learn about it, and at least try to apply the practices that you believe in when it makes sense to do so. You don’t really need to apply them all (though you will get bonus points from the cool kids if you do so, or at least pretend to), but there are a few of them that everyone really should do regardless of which agile variant you subscribe to.

So there you have it… try those books out in that order, and don’t forget to thank me later

Every time i see code with properties that return List<T>, IList<T>, ICollection<T> or any other of the types which allow you to modify the contents of a collection, i cringe a little. In most cases, the only reason to expose the contents of the collection is for read-only usage. You want people to be able to use the objects in the collection, but why on earth do so many developers continuously enable someone else to modify the internal state of the collection that they are holding?

The problem that i have with exposing a collection’s values through a type which enables other pieces of code to modify the state of that collection is that it is effectively a pretty big breach of encapsulation. You no longer have sole control over the contents of the collection. Anyone can effectively add and remove elements from the collection, or even clear them entirely, without your object knowing about it. Obviously, this can cause subtle side-effects in the behavior of your object. Which can (and sooner or later will) lead to some quality time between you and your debugger. You also no longer have the ability to easily change the type of collection you’re using which might prevent certain future improvements that you can make to a class. And in case you’re wondering why you’d want to change the type of collection, check out an example where the benefit was huge.

In a lot of cases, people do this because they don’t know any better or because it’s the easiest thing to do. I can’t tell you how many times i’ve seen people expose List<T> simply so that some calling code could use the ForEach method of that list. How long does it take to write a ForEach extension method for IEnumerable<T>? Granted, it should’ve been included in the .NET framework already but just because it’s not doesn’t mean you can’t do so.

Here’s my philosophy on exposing collections: if i’m writing a class that is responsible for holding a collection, i take away every ability to directly modify the contents of the collection. I typically expose the collection through an IEnumerable<T> property, and if necessary i will provide a specific AddXxx and RemoveXxx method. That’s it. You can do every single thing you need to do with the collection through IEnumerable<T>’s extension methods and through the AddXxx and RemoveXxx methods that i provide. Meanwhile, i have every ability to choose the collection type that i want to use, or to modify it later on. Furthermore, i can eliminate the possibility of side-effects in my code due to unexpected externally caused mutations in the state of the collection.

The only situation in which i think it’s OK to return a real collection type is if you have a method which is responsible for creating a list, but is not supposed to hold on to it. In that case, the created collection is clearly intended to be used in whatever way makes sense by the consumer. After all, you created the collection specifically for the consumer so they can do whatever they want with it. But if you need to hold a reference to the collection for some reason, then you probably also have more responsibilities than merely creating the collection. In that case, the collection is yours and you should encapsulate it as such.

I was talking to a friend the other day about prefixing instance fields. His opinion was that by prefixing instance field names (with an underscore for example) you have the benefit of immediately showing the difference in scope between the usages of a local variable in a method, or the usage of an instance field.

There is indeed a legitimate possible problem here: any possible mutation of an instance field in a method can have an intended or unintended impact somewhere else in the class since the mutation will outlive the duration of the method call. So it is important to be aware of this and that you are entirely sure that any possible mutation is actually intended and that no unintended mutations will occur because of some faulty code in a method. I don’t however think that prefixing field names is the best solution to avoid this problem. There are two practices that i recommend to easily avoid unintended mutations of fields in methods.

The first is to use as few fields as possible. To avoid any misunderstandings: i’m not saying that you shouldn’t use fields! I’m just saying that fields need to justify their existence. Fields inherently make up the state of an object, so if you don’t need a particular piece of data to hang around longer than say, the duration of a method call, then don’t put that data in a field. If you need to access that data in another method that will be called from the originally called method you can simply pass it to the other method with a method parameter. If you notice that you’re frequently passing the same pieces of data to a couple of methods, then you probably need to introduce a parameter object. Simply put: do not use fields unless the data you want to store in them are an inherent part of the object and the data needs to hang around for a while.

The second practice is too keep your methods short and focused. If you’re working with large methods, then i can certainly understand why you would want all the help you can get to easily spot mutations of fields. There’s only one question: why are you working with large methods? There are truly very few legitimate reasons for not splitting up large methods into separate reusable methods. When you use many small methods instead of large methods, it’s much easier to keep your focus on what any particular piece of code is supposed to do, and what it’s really doing. And if your methods are short, you will very easily see when a field is being used instead of a local variable.

The combination of these two approaches leads to a lot less unintended mutations of fields. And as a bonus, you get a lot more readable and maintainable code as well. If however you still like to get some visual help in identifying field usage in methods, you can enable the Color Identifiers feature in Resharper (available in the Options window under Code Inspection – Settings – Color identifiers) which will result in fields being represented with a different color from regular variables.

What do you think about this? Do you still prefer to prefix field names, and if so, why?