Posted on April 17th, 2011 154 comments
At work, our tech lead (architect on an agile project) recently asked every developer of the team, under the form of little 1 to 1 meetings, what in his opinion could be improved. Because every developer has his own vision of the project, knows issues that others might not be aware of, precious information shuch as those points below can be revealed :
- What are the main blocking points that we should get rid of
- Suggestions for areas to refactor
- Performance issues
The tech lead basically wrote down on a sheet of paper what the developer said (at first, without showing him what other colleagues have said, to not influence his opinion). He added marks in front of every point that was mentionned several times (let’s say that your colleague said previously that the UI styles really need to be refactored and that you say the same, he’ll put a mark in front of that remark so he knows that’s a point of importance and that those having several marks should be tackled first.
Then, once the developer has finished, he showed him the sheet with the results so the developer could react based on what others have said, like “oh yeah I forgot that point, it’s really important to address this one !”.
Those 1 to 1 meetings didn’t take longer than 30min in total and really shed light on very important points that enabled to increase the quality of our project. Very simple to do and very useful. Actually it’s the first project where I see this practice, this should be done from time to time on every project in my opinion.
Make good use of it
Posted on April 17th, 2011 67 comments
Almost every project has recurring operations consisting of several steps. The problem when there’s a lot of steps is that you have to know them by heart otherwise your project may become inconsistent and can lead to unexpected behaviors. Typically, developers save checklists files or screenshots of configuration settings on their local drive (like I did) to be sure they remain consistent and they don’t forget steps. So why not just sharing that precious knowledge with the team ?
Very simple : just create a Checklists.txt file at the root of your project (e.g. the backend project). As it will be source-controlled, any developer can add his contribution : add new checklists, complete existing ones or correct them.
Here’s an example on how to organize that file :
Enforcing that practice also ensures faster adoption of code base for new developers as it will act as a mini tutorial of the most important operations on your project !
Posted on August 9th, 2010 No comments
At the company I’m currently working for, there’s a big poster on the wall with the following headline : “if you see something like this, please raise your hand to improve code quality”. Below is the content of that poster – a list of things to avoid when coding – from Martin Fowler’s refactoring book.
- Duplicated code : the same code structure in two or more places is a good sign that the code needs to be refactored: if you need to make a change in one place, you’ll probably need to change the other one as well, but you might miss it
- Long methods : longs methods should be decomposed for clarity and ease of maintenance
- Large class : classes that are trying to do too much often have large numbers of instance variables. Sometimes groups of variables can be clumped together. Sometimes they are only used occasionally. Over-large classes can also suffer from code duplication
- Long parameter list : long parameter lists are hard to understand. You don’t need to pass in everything a method needs, just enough so it can find all it needs
- Divergent change : software should be structured for ease of change. If one class is changed in different ways for different reasons, it may be worth splitting the class in two so each one relates to a particular kind of change
- Shotgun surgery : if a type of program change requires lots of little code changes in various different classes, it may be hard to find all the right places that do need changing. Maybe the places that are affected should be all brought together into one class
- Feature envy : this is where a method on one class seems more interested in the attributes (usually data) of another class that in its own class. Maybe the method would be happier in the other class
- Data clumps : sometimes you see the same bunch of data items together in various places: fields in a couple of classes, parameters to methods, local data. Maybe they should be grouped together into a little class
- Primitive obsession : sometimes it’s worth turning a primitive data into a lightweight class to make it clear what it is for and what sort of operations are allowed on it (eg creating a date class rather than using a couple of integers)
- Switch statements : switch statements tend to cause duplication. You often find similar switch statements scattered through the program in several places. If a new data value is added to the range, you have to check all the various switch statements. Maybe classes and polymorphism would be more appropriate
- Parallel inheritance hierarchies : in this case, whenever you make a subclass of one class, you have to make a subclass of another one to match
- Lazy class : classes that are not doing much useful work should be eliminated
- Speculative generality : often methods or classes are designed to do things that in fact are not required. The dead-wood should probably be removed
- Temporary field : it can be confusing when some of the member variables in a class are only used occasionally
- Message chains : a client asks one object for another object, which is then asked for another object, which is then asked for another one, etc. This ties the code to a particular class structure
- Middle man : delegation is often useful, but sometimes it can go too far. If a class is acting as a delegate, but is performing no useful extra work, it may be possible to remove it from the hierarchy
- Inappropriate intimacy : this is where classes seem to spend too much time delving into each other’s private parts. Time to throw a bucket of cold water over them !
- Alternative classes with different interfaces : classes that do similar things, but have different names, should be modified to share a common protocol
- Incomplete library class : it’s bad form to modify the code in a library, but sometimes they don’t do all they should do
- Data class : classes that just have data fields, and access methods, but no real behavior. If the data is public, make it private !
- Refused bequest : if a subclass doesn’t want or need all of the behavior of its base class, maybe the class hierarchy is wrong
- Comments : if the comments are present in the code because the code is bad, improve the code