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
Posted on October 1st, 2009 No comments
The last Visug session was presented by Brad Abrams, Program Manager on the .NET Framework team at Microsoft, who shared with us guidelines about framework design. Member of the .NET Framework team since the beginning, his experience in the field is very valuable for architects and software designers, but also for developers who can highly benefit from a lot of good practices.
Below, you can find slides of the presentation :
Have a nice reading
Posted on April 5th, 2009 No comments
The List<T> class is the generic equivalent of the ArrayList class. It implements the IList<T> generic interface using an array whose size is dynamically increased as required and can be useful in many contexts. But you should avoid using it in public signatures. Why is it so ?
- The List<T> has too many members that makes it too heavy for many scenarios. Indeed, you might not want to return a List<T> collection with all its richness in all situations. According to some people, it’s even a violation of the Single Responsibility Principle
- While the List<T> class is not unsealed, it’s not specifically designed to be extended. Indeed, the class has no protected and virtual methods to allow beheviour altering through inheritance.
What should you use then ? It’s recommended that you use IList<T> or derived interfaces in public signatures.