Friday, October 5, 2007

Lessons learned

My current project is nearing completion of Phase 1. Along the way, I've compiled a list of observations, lessons learned, etc, some of which I've already blogged. Here are a few more:

1) When possible, use project references instead of file references. It may seem convenient to break up the code into smaller solutions and use file references across solutions. Problems arise when you have one master solution that builds all of the projects - you must explicitly set the build order. With project references, VisualStudio determines the proper order for you, meaning less maintenance. Also, when you right-click on a method/class/whatever and choose "Go To Definition," file references will take you to a page of metadata instead of the actual code.

2) Refactor often, and sooner rather than later. Say you're working on set of classes that use a common interface. You then decide to build a base class from that interface, and derive the other classes from the base. Don't leave old classes as-is and use the base only for new classes. One issue is you potentially leave bugs that the base was specifically designed to address. Another is that when a new team member joins the project, he will likely use existing code as a template for adding new functionality. If he sees the non-base-derived class and builds his own directly from the interface, refactoring later becomes more difficult.

3) Remove dead code. When you replace one method with another, delete the old method. When functionality is no longer required, delete that functionality. Don't leave it in. Don't comment it out. If you need that code later, pull it from your source repository. As the source grows in size, you'll have enough to deal with without adding the extra hassle associated with dead code.

4) Obsolete code if you can't currently delete it. Say you replace a method with another, but you can't replace all of the method calls at the moment (this especially happens with public methods.) Mark the method as obsolete and note the proper method to use instead. In C# this looks like [Obsolete("Use method foo instead")].

5) When changing the database schema, change the data access code at the same time. Until these two are in sync, your code doesn't work. You should have failing unit tests to flag the issues, but that's not always the case. In that instance, your first indication that there is a mismatch is when a developer attempts to run code that he thought was working. Often, the developer attempts to track down the problem, which another dev already knew about. All of which leads to wasted time.

6) Reduce confusion within the code. Maybe there's a method being incorrectly called, or called when another should have been used instead. It's not enough to correct the developer. Look at why the error occurred. Perhaps better comments on the method would help (use xml comments to populate intellisense.) Or perhaps the method could be named better. The problem may also be due to poorly architected code in need of refactoring. Bottom line: fix the issue instead of addressing symptoms.

7) If you can't unit test the entire codebase, write tests for the error-prone code. Ideally you want 100% code coverage from your unit tests. In reality this isn't going to happen. Therefore, focus unit tests on problematic and/or complex areas of code. We have lots of code binding to dataset columns. These columns are taken directly from the database. If a column was dropped from the table, you won't receive a compiler error on row["DroppedColumn"] but you will receive a runtime exception. These issues need to be caught by the unit tests, not the QA team.

8) Code generation must be handled very carefully. Most see code generation as an easy way to save time on a project. This is especially true for database access code - basic CRUD operations don't change from one table to the next. Once you have the first generation complete, how you proceed becomes critical. Say you spend a few weeks writing code that uses the autogen code. Someone then decides to make changes to the templates and regen everything. While the intent may have been valid, this regen quite likely broke existing code. At a minimum, your unit tests fail and you can easily find and fix all of the problems. But even then, time must be spent on the fix. If you don't have a decent set of unit tests - be prepared for the increase in tickets from QA. Unless your autogen code is truly separate from the rest of the project, it's probably better in the long run to gen once and modify by hand after that.

No comments:

Post a Comment

Note: Only a member of this blog may post a comment.