450 likes | 630 Views
This Code Stinks. An Oenophiles Guide to Bad Code Smells. General Context. How many times have you heard of exploits related to buffer overflows Or code bloat Or code inefficiencies At last years WatITis a speaker mentioned they used refactoring in one of their projects
E N D
This Code Stinks An Oenophiles Guide to Bad Code Smells
General Context • How many times have you heard of exploits related to buffer overflows • Or code bloat • Or code inefficiencies • At last years WatITis a speaker mentioned they used refactoring in one of their projects • How many here have written code and/or would like to write good/better code? WatITis | Strengthening Collaboration | December 8, 2009 | This Code Stinks
This Talk will cover: • Watitis • Principles • Bad code smells • And that oenophile thing WatITis | Strengthening Collaboration | December 8, 2009 | ***Program Title***
Watitis? “Refactoring is a disciplined technique for restructuring an existing body of code, altering its internal structure without changing its external behavior. Its heart is a series of small behavior preserving transformations. Each transformation (called a 'refactoring') does little, but a sequence of transformations can produce a significant restructuring. Since each refactoring is small, it's less likely to go wrong. The system is also kept fully working after each small refactoring, reducing the chances that a system can get seriously broken during the restructuring.” WatITis | Strengthening Collaboration | December 8, 2009 | This Code Stinks
Formal Definition • Refactoring (noun): a change made to the internal structure of software to make it easier to understand and cheaper to modify without changing its observable behaviour • Refactoring (verb): to restructure by applying a series of refactorings without changing its observable behaviour • Soyou might spend hours refactoring during which you might apply many individual refactorings • Refactoring came out of the Smalltalk environment in the 1980’s WatITis | Strengthening Collaboration | December 8, 2009 | ***Program Title***
Principles of RefactoringWhy? • Why Should you Refactor?Refactoring: • Improves the design of the software • As people change code – the code loses structure • The harder it is to see the design in the code, the harder it is to preserve it • Refactoring tidies up the code • Makes software easier to understand • When writing programs one often ignores the future developer – which could be oneself. Refactoring also helps in understanding unfamiliar code WatITis | Strengthening Collaboration | December 8, 2009 | This Code Stinks
Why? • Helps you find bugs • Clarifying program structure allows one to be more effective in writing robust code and avoid buffer overflows • Helps you program faster • Yes, refactoring improves quality. But this reduces the speed of development • Refactoring, by bettering the design, allows one to develop more rapidly and stops the design from decaying WatITis | Strengthening Collaboration | December 8, 2009 | This Code Stinks
Principles of Refactoring:When? • When should you refactor? Refactor when: • You add function • Adding new features • You need to fix a bug • When you get a bug report it was a sign you need refactoring since the code was not clear enough for you to spot the bug • As you do a code review • Code reviews help spread knowledge through development teams • Very important in writing clear code • Refactoring helps in reviewing someone else’s code WatITis | Strengthening Collaboration | December 8, 2009 | This Code Stinks
Problems with Refactoring • Databases • Some applications are tightly coupled to database schemas that support them. Also dependencies between schemas and object models means that changing the schema forces a migration of data • Changing interfaces • If refactoring changes an interface you have to retain both the old and new interfaces. • Design changes • Consider how difficult it would be refactor from one design to another • Deadlines • Productivity gains would appear only after the deadline and thus be too late WatITis | Strengthening Collaboration | December 8, 2009 | This Code Stinks
Bad Code Smells • If it stinks, change it. - child rearing philosophy WatITis | Strengthening Collaboration | December 8, 2009 | This Code Stinks
Duplicated Code • Number one in the stink parade • An example would be if you have the same expression in two methods of the same class, or the same expression in two sibling subclasses • Use • Extract Method, • Pull Up Field, • Form Template Method, • Substitute Algorithm WatITis | Strengthening Collaboration | December 8, 2009 | This Code Stinks
Long Method • The longer the method the harder it is to see what it’s doing. • Use: • Extract Method • Replace Temp with Query • Introduce Parameter Object • Preserve Whole Object • Replace Method with Method Object WatITis | Strengthening Collaboration | December 8, 2009 | This Code Stinks
Large Class • A class that is trying to do too much can usually be identified by looking at how many instance variables it has. When a class has too many instance variables, duplicated code cannot be far behind. • Use • Extract Class • Extract Subclass WatITis | Strengthening Collaboration | December 8, 2009 | This Code Stinks
Long Parameter List • Don't pass in everything the method needs; pass in enough so that the method can get to everything it needs. • Use • Replace Parameter with Method • Preserve Whole Object • Introduce Parameter Object WatITis | Strengthening Collaboration | December 8, 2009 | This Code Stinks
Divergent Change • Occurs when one class is commonly changed in different ways for different reasons. Any change to handle a variation should change a single class. • Identify everything that changes for a particular cause and use Extract Class to put them all together. WatITis | Strengthening Collaboration | December 8, 2009 | This Code Stinks
Shotgun Surgery • The opposite of Divergent Change. A change results in the need to make a lot of little changes in several classes. • Use Move Method and Move Field to put all the changes into a single class. Often you can use Inline Class to bring a whole bunch of behaviour together. WatITis | Strengthening Collaboration | December 8, 2009 | This Code Stinks
Feature Envy • Often a method that seems more interested in a class other than the one it's actually in. In general, try to put a method in the class that contains most of the data the method needs. • Use Move Methodbut you may need to use Extract Method first, then Move Method WatITis | Strengthening Collaboration | December 8, 2009 | This Code Stinks
Data Clumps • Clumps of data items that are always found together. • Turn the clumps into an object with Extract Class then continue the refactoring with Introduce Parameter Objector Preserve Whole Object WatITis | Strengthening Collaboration | December 8, 2009 | This Code Stinks
Primitive Obsession • Use small objects to represent data such as money (which combines quantity and currency) or a date range object • Use Replace Data Value with Object,Replace Type Code with Class, Replace Type Code with Subclasses,Replace Type Code with State/Strategy, • If you have a group of fields that should go together, use Extract Class • If the primitives occur in a param lists use Introduce Parameter Object • When working with an array consider Replace Array With Object WatITis | Strengthening Collaboration | December 8, 2009 | This Code Stinks
Switch Statements • The problem of switch statements is essentially one of duplication. You often find the same switch statement scattered throughout the program. • Use Replace Conditional with Polymorphism, Replace Type Code with Sublasses, Replace Type Code with State/Strategy, Replace Parameter with Explicit Methods, Introduce Null Object WatITis | Strengthening Collaboration | December 8, 2009 | This Code Stinks
Parallel Inheritance Hierarchies • A special case of Shotgun Surgery. Every time you make a subclass of one class, you also have to make a subclass of another. • Use Move Method and Move Field to combine the hierarchies into one. WatITis | Strengthening Collaboration | December 8, 2009 | This Code Stinks
Lazy Class • Classes that aren't doing enough should be refactored away. • Use Collapse Hierarchy, Inline Class WatITis | Strengthening Collaboration | December 8, 2009 | This Code Stinks
Speculative Generality • Don't over-generalize your code in an attempt to predict future needs. If you have abstract classes that aren't doing much • Use Collapse HierarchyRemove unnecessary delegation with Inline ClassMethods with unused parameters - Remove ParameterMethods named with odd abstract names should be brought down to earth with Rename Method WatITis | Strengthening Collaboration | December 8, 2009 | This Code Stinks
Temporary Field • Sometimes you see an object in which an instance variable is set only in certain circumstances. • Use Extract Class, Introduce Null Object WatITis | Strengthening Collaboration | December 8, 2009 | This Code Stinks
Message Chains • This is the case in which a client has to use one object to get another, and then use that one to get to another, etc. Any change to the intermediate relationships causes the client to have to change. • Use Hide DelegateOr try using Extract Method and then Move Method to move it down the chain. WatITis | Strengthening Collaboration | December 8, 2009 | This Code Stinks
Middle Man • When a class is delegating almost everything to another class, it may be time to refactor out the middle man. • Use Remove Middle Man. • If only a few methods aren't doing much, use Inline Method • You could also consider turning the middle man into a subclass with Replace Delegation with Inheritance WatITis | Strengthening Collaboration | December 8, 2009 | This Code Stinks
Inappropriate Intimacy • Two classes are overly intertwined. • Use Move Method,Move Field,Change Bidirectional Association to Unidirectional Association,Extract Class,Hide Delegate, Replace Inheritance with Delegation WatITis | Strengthening Collaboration | December 8, 2009 | ***Program Title***
Alternative Classes with Different Interfaces • Use Rename Method on any methods that do the same thing but have different signatures for what they do. You might have to use Move Method to move behaviour to the classes until the protocols are the same. WatITis | Strengthening Collaboration | December 8, 2009 | ***Program Title***
Incomplete Library Class • Library builders have a really tough time of predicting all that you want it to do and it is usually impossible to modify a library class later. • Use Introduce Foreign Method or if there is a lot of extra behaviour you might need Introduce Local Extension WatITis | Strengthening Collaboration | December 8, 2009 | ***Program Title***
Data Class • Classes with fields and getters and setters and nothing else (aka, Data Transfer Objects - DTO) • Move in behaviour with Move Method WatITis | Strengthening Collaboration | December 8, 2009 | ***Program Title***
Refused Bequest • Subclasses don't want or need everything they inherit. The Liskov Substitution Principle (LSP) says that you should be able to treat any subclass of a class as an example of that class. Most of the time that's fine, just don't use what you don't need. Occasionally you'll need to create a new sibling class and use Push Down Methodand Push Down FieldThe smell is worst if a subclass is reusing behaviour but does not want to support the interface of the superclass. In this case use Replace Inheritance with Delegation WatITis | Strengthening Collaboration | December 8, 2009 | This Code Stinks
Comments • Should only be used to clarify "why" not "what".Can quickly become verbose and reduce code clarity. • Use Extract Method, Rename Method, Introduce Assertion WatITis | Strengthening Collaboration | December 8, 2009 | This Code Stinks
Extract Method Before After void printOwing() { printBanner(); printDetails(getOutstanding()); } void printDetails (double outstanding) { System.out.println ("name: " + _name); System.out.println ("amount " + outstanding); void printOwing() { printBanner(); //print details System.out.println("name: " + _name); System.out.println ("amount “+ getOutstanding()); } WatITis | Strengthening Collaboration | December 8, 2009 | ***Program Title***
Rename Method Before After getlnvoiceableCreditLimit getinvcdtlmt WatITis | Strengthening Collaboration | December 8, 2009 | ***Program Title***
Move Method Before After class Project { Person[] participants; boolean participate(Person x) { for(inti=0; i<participants.length; i++) { if (participants[i].id == x.id) return(true); } return(false); } } class Person { int id; } ... if (p.participate(x)) ... class Project { Person[] participants; } class Person { int id; boolean participate(Project p) { for(inti=0; i<p.participants.length; i++) { if (p.participants[i].id == id) return(true); } return(false); } } ... if (x.participate(p)) ... WatITis | Strengthening Collaboration | December 8, 2009 | ***Program Title***
An Oenophiles Guide to Bad Code Smells • Based on paper that related the empirical study of bad code smells by means of an olfactory mapping to the aromas of Le Nez Du Vin (nose of the wine). The aim of this mapping was to find whether an olfactory response to code refactoring can lead to better code generation. Findings indicate that this aroma mapping can benefit the subjective evaluation of code and can be extended to data evaluation. WatITis | Strengthening Collaboration | December 8, 2009 | ***Program Title***
In the table are outlined the 22 bad smells identified by Fowler in column one with a small description of the bad smell in column two. The entries in the table act as a guide in identification of areas of bad code design. • Automatic refactorers such as Refax and jRewritercan be used to find bad code smells in programs. Commercial and open-source tools such as XRefactory, ReTool by Chive, or jFactor by Instantiations are available as add-ons to programming environments. However, they do not analyze when a certain refactoring can be applied. • Finding these areas of bad smell is more related to human intuition than to an exact science. The next slide introduces column three and the basis of the empirical study. WatITis | Strengthening Collaboration | December 8, 2009 | ***Program Title***
Odours and Memory • The olfactory system is one of the oldest and most vital parts of the brain. Olfactory information migrates not only to the limbic system – primitive brain structures that govern memory storage, behaviour, and emotions – but also to the brains cortex where conscious thought occurs. • The human sense of smell and memory are closely linked. This sense evokes memories. Research has shown that odour memory falls off less rapidly than that of other sensory memory. Also, the “Proust effect” states that when an odour is associated with experience a similar odour can recall the memory. The sense of smell is better at this memory cue effect than the other senses. • So, how to produce an olfactory experience that can be used in the detection of bad code smells? It was necessary to first find a standard set of aromas that could be used in the experiment. WatITis | Strengthening Collaboration | December 8, 2009 | ***Program Title***
Le Nez du Vin • First find a standard set of aromas that could be used in the experiment. • The Le Nez du Vin aromas were used since they are a standard collection of 54 aromas readily available and commonly known. There is also a collection of wine fault aromas which were more appropriate since we are dealing with bad smells. It is interesting that with the 12 Les Défauts you can learn how wine faults can be eradicated or their effects lessened. Since there were only 12 fault aromas, 10 more were borrowed from standard 54 aromas to make a one-to-one relationship with the bad code smells. • What becomes of the other 44 aromas? It was considered to pair a fault aroma such as moldy earth with that of a sweet aroma such as banana. As bad smells were repaired the good smells would predominate and act as reward. However, this segment of the study is left to future considerations. WatITis | Strengthening Collaboration | December 8, 2009 | ***Program Title***
Pain and Pleasure • What becomes of the other 44 aromas? It was considered to pair a fault aroma such as moldy earth with that of a sweet aroma such as banana. As bad smells were repaired the good smells would predominate and act as reward. However, this segment of the study was left to future considerations. WatITis | Strengthening Collaboration | December 8, 2009 | ***Program Title***
Why? • The purpose was to evaluate the effectiveness of an olfactory mapping of aromas to bad code smells. This was accomplished ,be it on a small scale, by having participants view bad code and smell the aromas. The conclusions were positive as a second experiment weeks later with unknown bad smells was used and participants were able tell what faults were present before viewing the code. How this would extend to all 22 bad smells is open to further investigations. • An olfactory reaction is a subjective response to chemical stimuli. As we age this response decreases. However, memories of odours still remain strong. WatITis | Strengthening Collaboration | December 8, 2009 | ***Program Title***
Future Considerations • Since aromas can be synthesized, it would be appropriate to produce a unique set specific to bad code smells instead of relying on existing ones such as those mentioned previously. This can lead to automatic refactoring programs producing an olfactory signature of the code that can assist developers. Similar to what jCOSMO currently does for visualization but with better memory retention by the developer. • Possible extensions of this study are to use aromas to gage the overall health of a computer system similar to that of checksums. Also, to gage the fitness of large data sets and to actually put value on, or an olfactory response to the statement “this data does not smell right.” WatITis | Strengthening Collaboration | December 8, 2009 | ***Program Title***
Conclusion • Look forward to a SAW course/workshop on code refactoring • It would be nice in the future to uncork a vintage code to see if it still smells fine. WatITis | Strengthening Collaboration | December 8, 2009 | ***Program Title***
Automated Code Refactorers Many software editors and IDEs have automated refactoring support. Here is a list of a few of these editors, or so-called refactoring browsers. • IntelliJ IDEA (for Java) • Eclipse'sJava Development Toolkit (JDT) • NetBeans (for Java), http://profiler.netbeans.org/download/ • Bicycle Repair Man (for Python, works with emacs and vi), http://bicyclerepair.sourceforge.net/ • Visual Studio (for .NET) • ReSharper (An addon for Visual Studio), http://www.jetbrains.com/resharper/ • Refactor Pro (An addon for Visual Studio), http://www.devexpress.com/Products/Visual_Studio_Add-in/Refactoring/ • Visual Assist (An addon for Visual Studio with refactoring support for VB, VB.NET. C# and C++) • DMS Software Reengineering Toolkit (Implements large-scale refactoring for C, C++, C#, COBOL, Java, PHP and other languages), http://www.semdesigns.com/Products/DMS/DMSToolkit.html • Photran a Fortran plugin for the Eclipse IDE, http://www.eclipse.org/photran/ • jCOSMO, An extendible Java code smell detector, http://www.cwi.nl/projects/renovate/javaQA/ • CCFinder and associated tools – a token base clone detection tool, http://www.ccfinder.net/ • Jrewriter, A new Java static analyzer with automatic refactoring capabilities, http://www.jrewriter.com • Polishing Ruby: Refax the automatic refactoring engine, http://blog.zenspider.com/archives/2005/02/refax_the_autom.html WatITis | Strengthening Collaboration | December 8, 2009 | This Code Stinks
References Fowler, Martin (1999). Refactoring. Improving the Design of Existing Code. Addison-Wesley. ISBN: 0-201-485672. Wake, William C.(2003). Refactoring Workbook. Addison-Wesley. ISBN 0-321-10929-5. Feathers, Michael C (2004). Working Effectively with Legacy Code. Prentice Hall. ISBN 0-13-117705-2. Kerievsky, Joshua (2004). Refactoring To Patterns. Addison-Wesley. ISBN 0-321-21335-1. Arsenovski, Danijel (2008). Professional Refactoring in Visual Basic. Wrox. ISBN 0-47-017979-1. Arsenovski, Danijel (2009). Professional Refactoring in C# and ASP.NET. Wrox. ISBN 978-0470434529. http://www.refactoring.com/ WatITis | Strengthening Collaboration | December 8, 2009 | This Code Stinks