Monday, December 14, 2009

Coding conventions

There are several small things, that usually at the start of a software project nobody cares of, and the project becomes messed with inconsistencies because of them. The coding conventions were created for this, but there are project specific rules which cannot be put in a cross-project list of coding conventions. It would be good if these rules would be agreed upon at the start of the project and followed. An example of what needs to be decided by these rules (WPF):

I. Projects/folders:
  1. Agree upon how the solution is split into projects. Don't create pointless dlls and exes. Create new projects only if (a) it is impossible to implement the functionality without a new project - for example the solution consists of a server located somewhere and multiple clients, so that you need a web service project and a windows exe client project - or (b) they are really reusable - for example if you implement a general purpose complex control which does not rely on your data objects at all, for example a general purpose timeline control - or (c) creating them separates the features, and thus the developers from each other, one developer works only on project 1, and the other only on project 2, if they work on both, having two projects just introduces clutter. Don't create unneeded projects for the layers of abstraction.
  2. Agree upon how the code in a project is split into folders. Create folders by functionality, not by levels of abstraction. If you have Feature1Layer1, Feature1Layer2, Feature2Layer1, Feature2Layer2, create a Feature1 and a Feature2 folder. If you have 10 features with 10 files each, creating a folder for each layer either ends up with a folder with lots of files, or a subfolder for each feature within the layer folder. And a developer usually modifies more than one layer of a feature, so having by feature instead of by layer folders is much more comfortable to navigate. And also, for a new developer added to a project, is easier to see the implementation of a feature - thus the project's architecture - and learn how to implement his feature based on that. Of course there is functionality that is common among features, that needs its own top level folder.
  3. Don't duplicate data layer classes with minor functionality differences in each feature. The data layer needs to be standardized within the project. Have a single set of data classes. Having duplicates creates conversion headaches besides having more code to understand/modify.
II. Cs:
  1. Agree on the member ordering in the file. For example fields, events, constructors, then the event handlers in the order in which they occur, and the order of the controls in the UI, putting the helper method just below the method it helps, not after all the event handlers. Having the helpers exactly below the method it helps avoids a lot of scrolling in the source file, because when you work on/want to understand a method, you work on/want to understand its helpers too.
  2. Agree upon whether private should be written in front of the private members in the cs file.
  3. Agree on the field naming. Shoud it be _name, name or name_?
  4. Agree whether automatic properties (public string Name { get; private set; } ) should be used instead of public fields. Using properties just makes the members have the same icon in intellisense and allows data binding.
  5. Use the default Visual Studio formatting, Ctrl + K + D.
III. Validation and error handling:
  1. Log the unhandled exceptions of the worker threads too with AppDomain.UnhandledException and show an error message to the user, showing the stack trace too within an initially closed expander. Eventually allow the user to continue from a failed dispatcher task (wndproc error) on the UI thread with Dispatcher.UnhandledException. This continuation must be considered thoroughly, because the program's data may be corrupted, and continuing can corrupt the program's database and make it unusable after a restart.
  2. Don't close a window before the action triggered by the window is fully executed, to don't lose the user's unsaved data in case of an error.
  3. Agree upon how error handling is done. Is it by showing a message box with a simple error text and a details expander showing the stack trace and logging the error to a log file? Put the code that can be shared into an abstract window class from which all dialog windows inherit.
  4. Agree upon how validation is done. Is it done when the user presses Ok, showing a message box and selecting the invalid control? Or on TextChanged/SelectedIndexChanged showing an ErrorProvider and disabling the Ok button? Show an example implementation. If there is code that can be shared, make the dialogs inherit from an abstract window class which defines the common error handling functionality.
IV. General UI functionality:
  1. Agree upon the casing of the text in each type of control, with examples. Window title, label, button, radio button, check box, group box title, combo box items, list box items, list view headers, list view items, etc.
  2. Focus the first control of the window when the window is loaded, so that the user does not have to press an extra tab. Put this focusing in an abstract window class from which all dialogs inherit, add a property to this window which specifies which control to focus, and set it in the constructor of the subclass.
  3. Set IsDefault on the Ok button and IsCancel on the cancel button.
  4. Agree upon the double click actions in the list views: open the edit window for example.
V. Xaml:
  1. Agree upon the naming of the UI controls, (a) should it be nameTextBox, tbName, _tbName, (b) whether Name or x:Name should be used, and (c) whether the controls should be marked as x:FieldModifier = private.
  2. Agree on the formatting of the xaml. For example, put multiple properties on the same line, indent the properties which do not fit on a single line to start below the first property of the previous line, put the properties in the order x:FieldModifier, Name, Content, Grid.Row, Grid.RowSpan, Grid.Column, Grid.ColumnSpan, Alignment, Margin, Padding, IsEnabled, Style, event handlers.
  3. Agree upon where the control texts are placed: mixed with the code, or within a resource file for localization.
VI. Styles:
  1. Create global - x:key-less - styles for each type of control when you don't use the default styles. For example, you can create a button style with Height = 22, MinWidth = 70 to have same width buttons, except the ones with very long caption.
  2. Create styles with x:key for the frequently used control variants. For example create a multiline text box style which specifies VerticalScrollBarVisibility = Auto and AcceptsReturn = true.
Those rules from above which need implementation should be assigned to somebody, otherwise either nobody cares about them, either time is wasted by more than one programmer thinking about them.

VII. Implementation notes:
  1. State goals in the spec by priority. If there is no priority for the goals, the programmer may want to simplify the complicated things, not knowing that it is complicated because that is the most important thing that the program does (i.e. that separates it from the competitors).
  2. State non-goals in the spec, to avoid over-complicating the design of the code by allowing the implementation of the non-goals, and to avoid a programmer thinking about how to implement them.
  3. Have clear specifications for the feature before starting to work on it. Otherwise developers waste time by thinking about how something should work, figure out something after a lot of thinking, and maybe that thing is not what the one in charge with the specs wants. Sooner or later each program will end up with a spec, because the testers need it and the program needs to be implemented. And if it is made earlier, and by the people who has the most experience and the most decision rights, less time is wasted on it. He may decide in 2 minutes a thing which for the programmer may take an hour to decide. But if it is not in the spec, the programmer may assume that the man in charge with the spec did not find an answer in half an hour, so it is reasonable for him to spend an hour on it.
  4. If you have a big project, put a programmer to document the design of the code, as his highest priority task.
  5. If you split the work by feature, and all the features have the same layers, implement one of the features from start to finish before starting to work on the other features, to avoid each of the developers thinking and solving the same problems again and again, wasting a lot of time and getting a lot of inconsistencies and code duplication. You can split this first task between the developers, each layer a developer, and work on it until the code is as easily reusable by the other features as possible.
  6. Split the rest of the features, so that the least amount of interaction between the developers is necessary. Put a single developer to a feature, or as few as possible. If your program has 2 features and you have 2 developers and you put both of them to work on feature 1 until it is finished, then on feature 2 until it is finished, both of them will need to understand both features' code. If you put a single developer onto a feature, he will need to understand only half the amount of code.
VIII. Bug fixing:
  1. Put a single developer to a feature, or as few as possible. Otherwise all the developers working on a feature will think about the same bugs of the feature, wasting time, and either none of them will fix the bug to don't conflict with others, either more than one people will fix the same bug.
  2. Fix the bugs before implementing new features. Otherwise time is wasted by having to put all the small bugs in the bug tracker to don't forget them and having the testers retest the feature just to make sure that fixing a bug that was known to the developer (even without any testers) did not break anything when fixed close to the delivery (release) date. Even if you avoid putting the small bugs into the bug tracker, you may waste the tester's time, because he may see the bug that you know of, and he will add it or search for it in the bug tracker.
IX. Working conditions:
  1. Make it easy for the programmers to install programs that they need. If they need to wait a whole day to download the installer or the virtual machine from a remote location, they either waste time with it (maybe downloading some wrong thing, and needing to start again) or give up. Same for keeping the install dvd-s hidden in an IT guy's office, and having to wait for him until the next day to find the dvd-s.
  2. Don't make security more important than having access to data. If the programmer has to ask permission for every little thing and wait for it for a few days and he maybe gets it, maybe not, he'll give up, and work less efficiently, not using those things which would make his work more efficient but for which's permission he would have to hassle.
  3. If the project involves peer to peer communication and has special requirements (for example Windows Vista or 7 and all of the computers in the company have Windows XP), allocate time explicitly for setting up the test environment if you want the developers properly test their work. Or better, have the system administrator set it up.
  4. The developers should have specs for the work that they're going to do in the next 5 days, to eliminate the unneeded stress of getting in the morning to the work and not knowing what you're going to work today, and whether you'll have a spec or not. This eliminates the time wasted for waiting for the on-the-fly spec too.
  5. Setting deadlines does not have any influence on the release date whatsoever. Tight deadlines worsens the software's architecture and introduces hard to find bugs. For example threading is not designed at the beginning correctly, with as few communication between the threads as possible, and race conditions are introduced: for example the program crashes on one computer but does not crash on the other, or crashes 5 times today, once tomorrow. And features are partially implemented, edge cases unhandled, just to respect the deadlines, and are left to be implemented in the bug fixing phase, when they are not assigned to the developer who wrote the buggy code, and even if it would, it is easier to implement a feature at once, than to go back 6 months later, don't remember the tricky bugfixes that you made 6 months ago, and break 2 things while fixing one. Realistic deadlines are theoretically good to know how complex a feature should be, that is how much to cut down from the non essential features. For example how specific the error messages should be. How good the error recovery should be. How detailed should the edge cases be handled. But even for that, a spec is much better, because the developer and the project manager/leader may have different ideas about what needs to be implemented and what should be left out.
  6. Allow time for learning. Nowadays each project requires a few technologies that the developers have never used before. And these technologies get obsolete before they can learn them well. If a developer uses Entity Framework, Sync Framework, Sql CE, WCF and Spring, all for the first time, he needs some time to learn them. Also, if he is new to the project, he needs some time to learn the code that he will be modifying. Not allowing time to learn at the beginning, with some simple test programs, he will introduce the bugs he would learn about in the 50k lines project, which is much harder to debug than a 50 lines test program. Scrum has the drawback that it doesn't allow the developers to learn the technologies before using them, because you must show that you implemented something in the real project. But it's hard to tell the customer that some of the developers have never used half of the tools we use on the project, and they need time to learn them.
  7. Plan the UI refresh at the beginning. For example specify what needs to be refreshed when I add a user. Write clearly specified refresh methods which everybody should use, to don't have performance problems doing the refresh many times after a single change.
  8. Split the project to people so that testers can identify whose code a bug is in. This way developers will write code that is easy to debug, with as few threads and race conditions as possible, and will put correct logging into it when necessary to debug it. That is, not just write out the method name on entering and exiting it, and release stack traces on an exception, but writing out the parameters of the important methods. All the spec writing and upfront planning don't look good in the customer's eyes either, he wants code asap, especially when he knows that a lot of time was already spent on the proposal and estimation.
  9. Allow time for setting up a one-step or as few steps as possible developer test environment. It is better to take some time at the beginning to automate these steps, than to repeat them over and over again, with possible errors, when trying to reproduce a bug. And not knowing if the person who reported the bug hasn't done one of these steps differently.
  10. Continuous integration is good to resolve conflicts occurring because of 2 developers modifying the same code, but it isn't always good. If you have a feature somewhere in a program that starts slowly possibly due to a useless recompile every time even if nothing has changed, needs starting a WCF service and entering a password, it may be faster to write your piece as a separate program and after debugging it, move it into the real thing. Scrum and one patch a day does not help with this either.
X. Links:
  1. Separate teams by functionality not activity
  2. The Joel test

No comments:

Post a Comment