With C# 3.0 came one new feature I both love and hate; the implicitly typed local variable: var. I think it is great because it is implicitly typed, but it is still strongly typed. At compile time it will be the explicit type as if you had typed the actual type name. Visual Studio is able to determine during development what the type is, so there isn't much of a downside. However, I believe it gets overused.
I think it is great if the line contains the "new" keyword, because I can already see the type name because it gets written for the constructor. Using "var" all the time really irks me though. It is inconvenient, because I can't always tell what type the variable is. This isn't always too much of an issue, but I prefer defining the type when possible.
For generics it is great since those type names get quite long. With generics we define two types together, so with longer names this sometimes becomes a little unwieldy. This is one of the great strengths of this new behavior in C#.
Earlier today I was reading an interesting post from Jimmy Bogard. He was blogging about The Filter-View anti-pattern. What bugged me about the post was his use of var. Now I know this has nothing to do with the actual post itself, but who can resist posting about their coding preferences?
In the post he had this code snippet.
public class ProductController : Controller
{
[LoginInfo]
public ActionResult Index()
{
var products = _productRepository.FindAll();
return View(products);
}
Now can you tell me what type is the variable products? Sure you might guess a collection of Product objects. You're probably right. Is it an IEnumerable? Is it an array? Is it a list? No, you don't know for certain. In fact you can't be sure it is a collection of Product objects. What if it were a collection of product names? That could be a badly named method and you get back strings.
You might think I am stretching a bit there on the strings, but there certainly could be methods which just didn't clarify well enough in there name. Now I know that I can mouse over the type in Visual Studio and instantly find out what type it is, but I should have to. In fact I probably wouldn't. I would probably try to use it and be kind of annoyed. Odds are I would have made an assumption on what type it is, and then I might have been annoyed when I tried to use Length thinking it was an array and then found our it was a List and I needed to use Count.
Anyway, I do recommend reading blog posts from Jimmy Bogard even though he overuses the implicitly typed local variable in his code snippets.
Visual Studio as well as many other applications which know what language you're using are able to do a lot of the little shortcuts for common tasks like commenting and uncommenting code. This can help speed things up a great deal since you can usually do many lines at a time. Also you don't have the context switching to go to a mouse to click the button.
Instead of clicking that button just type.
[Ctrl] + K + C - Comments out code
[Ctrl] + K + U - Uncomments code
It is not the best of keyboard shortcuts, however, it lets you avoid the context switching of moving to the mouse and when commenting out multiple lines it is quite quick. This will work in code such as C# as well as with the markup of XML-based languages.
A big issue which can be seen when looking at a lot of ASP.NET code is the classic "do everything" method. This is often some kind of an event handler. It is sometimes one for the page such as with Page Load. Other times it is a control on that page that owns the event. Either way this is a nasty piece of code whether you're testing or not.
These dangerous pieces of muck are difficult to work with, and I'll say they scare the crap out of me when I need to work with them. So I am sure everyone knows why they are difficult in general to work with. Obviously there is a lot of code there in one place doing a lot of different stuff.
When we think about what might be in one such method we come up with a lot of nasty stuff.
Nasty Stuff Sometimes Found in Large Event Handlers (Some of these are much worse than others)
- Business Rules Logic
- Dependencies on page controls and their properties
- Knowledge of the underlying data persistence layer
- Dependencies on the server context
- Dependencies on configuration information
- Complicated UI logic determining how the page should be rendered
So I am sure I've missed some other really nasty thing which could have been in this method. As with any large method this is obviously violating the Single Responsibility Principle. For the sake of this blog post I am going to define the method we are talking about as a click handling event for a save button on a web form.
So obviously we should extract a method from this to get the "Complicated UI logic determining how the page should be rendered" into a method which has anything to do with UI logic. Since we are on the page, we can keep the UI logic here. This is the UI layer after all.
Running through each of these and getting them into the right place could take us a while, so I'll just be covering a couple of them here.
Removing the Dependency on the Data Persistence
This is one of the most important steps we can take to improve our code quality. Removing the database dependency will assist us in keeping our concerns separated and shrinking the size of this monster method. By removing this dependency we are better able to test this code. We want to make sure that we are coding against an interface giving us access to the data we need. This will allow us to remove the dependency on our data when we are testing.
I don't care exactly how you do it, but this needs to be broke apart. I generally use the word "Repository" when referring to a class that will get and store my data. I like this word because it is a non-specific word meaning that it is a persistence class. There are a lot of implementations of repositories. If you want to learn more then read about repositories on Google. A lot of very smart people have written things for and against the repository pattern.
We want to use this pattern because the data accessing code and the persistence layer can be moved into the repository. This lets us remove that code from the nasty method.
When we write tests we will use a mock, fake, or stubbed version of the repository so we don't have to maintain a database.
Removing the Dependency on the Web
Since we are working with web forms here we have a strong dependency on our web form when we are in the event handler in the code behind of our page. In ASP.NET MVC this is not as much of an issue. A lot of people don't realize that a lot of the stuff that makes MVC so nice is that it is actually just helping to guide us towards a lot of these principles and rules we already tried to follow. This separation and everything is right out of the Single Responsibility Principle.
When we are breaking apart our code we want to have a lot of small interconnected pieces of code working together to achieve something great. However to get to that point we should be moving in small steps. One of the best ones in this circumstance is to pull as much of the logic as possible away from the web form.
To achieve this separation we will create a class we will call a "controller". If we make the class WebForm1Controller we can put a method in there that handles this event for us. It will, however, take in the values it needs from the Controls instead of having access to the controls itself. It can also take in any values it needs from the HttpContext or anything else. The point of this is that this code in the controller can run without actually having the web portion of your code. (THIS LETS YOU TEST IT!)
So you might be thinking at this point that all we have done is moved the code. Well sort of. What we did was limit our dependency on the web. We did this by making sure that all we needed was the values. You are right that we could have kept this as a separate method in the code behind. The difference is that in this new class we do not even have access to the controls so no one can try to directly access them from here. We are also more easily able to create instances of this class. The one associated with a web form has a lot more red tape to deal with than our freshly created controller class.
You might now ask if we are done fixing this code. Certainly not. We have much more to do, but the code is better. As I've said before, it is a lengthy process to write better code. If you're not working towards writing better code every time you write code, you're probably making code worse. Try making one thing cleaner, nicer, more concise every time you're working with the code. Even if you're just extracting a small method or renaming something, you're making things better. Don't be discouraged if you can't do a large refactoring in one sitting. Notice here that we aren't done, but we've made the code a lot better. In fact, I bet we could test the controller's business logic now.
Software development is not alone in the world. Surprised aren't you? There are other fields which exist. Most of these fields have been around much longer than we have, and sadly they tend to do things better than we do in a lot of ways. Heck they're much more mature than we are. I am not saying there is anything wrong with how we are doing things, but I do believe that our field can grow and develop a great deal still. We need to be looking at these other fields though and see how they do things. We need to take what they do and apply what we do well in order to become truly great.
A lot of people have read about "The Toyota Way". In fact it is on my reading list, and I plan on getting to it sometime soon. There is a lot than can be learned from other industries I believe. Lean thinking works no matter what field you're in. From the Wikipedia entry on this topic we can find this as "Principle 8".
Use only reliable, thoroughly tested technology that serves your people and processes.
Now can someone tell me what the 5th word is in this quote? Yes, that is correct. The word is "tested". Think about how cars are made here for a minute. Why don't we start at the high level and work our way down. We will start with our full system integration tests.
The car crash test: When test crashing a car there is no one behind the wheel. Why? I know of at least two reasons; it is dangerous and more importantly it needs to be identical every time. So what do they do? They have a test harness in place that allows them to automate the test yielding an identical test every time. They could have remote controlled the cars or something, but that would allow for too much human error.
Testing individual pieces: The different components of cars are tested for defects. Usually there is some machine that comes along and does something like applying pressures in a stress test. They could have a person go out and apply the same tests, but they don't. Why? If you're trying to have a consistent test it must be automated. Is that the only reason? Of course it is not. Automated tasks are also much, much faster. Machines are much faster than humans.
So aren't all of these machines expensive? Yes, of course they are. Don't you think it is expensive to build a machine that crashes cars? It is also expensive to have to crash perfectly good cars just to see how they respond in an accident.
Software test harnesses are not expensive. They are code. What happens if you try to make your code throw an exception while in a test harness? NOTHING. You run the code again.
Testing software is cheap. Other industries test there stuff as well. They aren't testing in the same way we do. Why? Because we have this huge advantage they don't. Our test harnesses are just software. Our stuff is cheap and reusable. We can run our tests all the time and the only resource we use is electricity. Much cheaper than the rubber and steel the car industry needs.
Have you ever heard another developer say, "it works on my machine"? Well I am sure a lot of people have heard that common phrase. Heck I've used it, but a lot of the time I will point out that it also works on the build server. That last part is what really counts in my opinion.
Any and all code you write should be checked into a source control repository. I think most people agree with that statement. Another very important thing is that your code actually work after being checked out from the repository. I wrote a blog post a while back about how to structure software projects.
One of the most important aspects of a well-organized project in my opinion, is the ability for a new developer on a project to be able to sit down and have a working solution as soon as they get the latest version of the application from the source control repository. Not only should that person be able to have a working solution, but that working copy should assist the developer in setting up the local environment. The solution should also be in a state which allows all of the project's tests to run without the user having to jump through a bunch of hoops.
I feel strongly that working on a project should be as simple as a checkout and a few more clicks to see the app running. Having a build server is a great way to maintain this on a project. Everything you need to be able to run your app needs to be in source control. This will be how the build server will get the application, so if it isn't in there, the server will not build it.
The build server is the ultimate answer to the question of, "does this code work on another machine?" You'll keep your code working everywhere, and you make sure that all of the pieces are integrated and working together effectively.
Have you ever had a developer commit their changes at the end of the day and commit code that breaks the build for everyone else? It is a lot harder to check in broken code without realizing it if the build server informs you right after checking in. Sometimes it is the compilation which fails and other times there are tests failing. It doesn't matter how it was broken, but it is broken now. Luckily you've got a build server to save you.
With a build server in place, everyone will know when the build breaks. If the offending developer is not around to fix it, you can always revert the changes until such time that the offender is available to fix the issue. Everyone continues on, because they knew of the issue and corrected it by reverting immediately. No one continued on thinking that broken code was really working code.