Topics covered:

  • Refactoring
  • Refactoring principles and tips
  • Code smells
  • Refactorings
    • Data level, statement level, method level, class level, system level refactorings, etc.
  • Refactoring patterns

Video (in Bulgarian)

Presentation Content

What is Refactoring?

Refactoring means "to improve the design and quality of existing source code without changing its external behavior". -Martin Fowler

  • It is a step by step process that turns the bad code into good code
    • Based on “refactoring patterns” → well-known recipes for improving the code

Code Refactoring

  • What is refactoring of the source code?
    • Improving the design and quality of existing source code without changing its behavior
    • Step by step process that turns the bad code into good code (if possible)
  • Why we need refactoring?
    • Code constantly changes and its quality constantly degrades (unless refactored)
    • Requirements often change and code needs to be changed to follow them

When to Refactor?

  • Bad smells in the code indicate need of refactoring
  • Refactor:
    • To make adding a new function easier
    • As part of the process of fixing bugs
    • When reviewing someone else’s code
    • Have technical debt (or any problematic code)
    • When doing test-driven development
  • Unit tests guarantee that refactoring does not change the behavior
    • If there are no unit tests, write them

Good Code Main Principles

  • Avoid duplication (DRY)
  • Simplicity – Keep it simple stupid (KISS)
  • Make it expressive (self-documenting, comments)
  • Reduce overall code (YAGNI)
    • More code = more bugs
    • Avoid premature optimization
  • Appropriate level of abstraction
    • Hide implementation details
  • Boy scout rule
    • Leave your code better than you found it
  • Don’t make me think
    • Code should surprise the reader as little as possible (principle of least astonishment)
    • Consistency!
  • Write code for the maintainer
    • Unit test
  • SOLID
    • Single responsibility principle
    • Open/closed principle
    • Liskov substitution principle
    • Interface segregation principle
    • Dependency inversion principle

Refactoring Process

  • Save the code you start with
    • Check-in or backup the current code
  • Make sure you have tests to assure the behavior after the code is refactored
    • Unit tests / characterization tests
  • Do refactorings one at a time
    • Keep refactorings small
    • Don’t underestimate small changes
  • Run the tests and they should pass / else revert
  • Check-in

Refactoring Tips

  • Keep refactorings small
  • One at a time
  • Make a checklist
  • Make a “later”/TODO list
  • Check-in/commit frequently
  • Add tests cases
  • Review the results
    • Pair programming
  • Use tools (Visual Studio + Add-ins)

Code Smells

  • Certain structures in the code that suggest the possibility of refactoring
  • Types of code smells
    • The bloaters
    • The obfuscators
    • Object-oriented abusers
    • Change preventers
    • Dispensables
    • The couplers

Code Smells: The Bloaters

  • Long method
    • Small methods are always better (easy naming, testing, understanding, less duplicate code)
  • Large class
    • Too many instance variables or methods
    • Violating Single Responsibility principle
  • Primitive obsession (overused primitives)
    • Over-use of primitives, instead of better abstraction
    • Part of them can be extracted in separate class and encapsulate their validation there
  • Long parameter list (in/out/ref parameters)
    • May indicate procedural rather than OO style
    • May be the method is doing too much things
  • Data clumps
    • A set of data items that are always used together, but are not organized together
      • E.g. credit card fields in order class
  • Combinatorial explosion
    • Ex. ListCars, ListByRegion, ListByManufacturer, ListByManufacturerAndRegion, etc.
    • Solution may be Interpreter (LINQ)
  • Oddball solution
    • A different way of solving a common problem
    • Not using consistency
    • Solution: Substitute algorithm or use adapter
  • Class doesn’t do much
    • Solution: Merge with another class or remove
  • Required setup/teardown code
    • Requires several lines of code before its use
    • Solution: Use parameter object, factory method, IDisposable

Code Smells: The Obfuscators

  • Regions
    • The intend of the code is unclear and needs commenting (smell)
    • The code is too long to understand (smell)
    • Solution: organize code, introduce a new class
  • Comments
    • Should be used to tell WHY, not WHAT or HOW
    • Good comments: provide additional information, link to issues, explain reasons, give context
    • Link: Funny comments
  • Poor/improper names
    • Should be proper, descriptive and consistent
  • Vertical separation
    • You should define variables just before first use
      • Avoid scrolling
  • Inconsistency
    • Follow the POLA
    • Inconsistency is confusing and distracting
  • Obscured intent
    • Code should be as expressive as possible

Code Smells: OO Abusers

  • Switch statement
    • Can be replaced with polymorphism
  • Temporary field
    • When passing data between methods
  • Class depends on subclass
    • The classes cannot be separated (circular dependency)
    • May broke Liskov substitution principle
  • Inappropriate static
    • Strong coupling between static and callers
    • Static things cannot be replaced or reused

Code Smells: Change Preventers

  • Divergent change
    • A class is commonly changed in different ways for different reasons
    • Violates SRP (single responsibility principle)
    • Solution: extract class
  • Shotgun surgery
    • One change requires changes in many classes
      • Hard to find them, easy to miss some
    • Solution: move method, move fields
    • Ideally there should be one-to-one relationship between common changes and classes
  • Parallel inheritance hierarchies
    • New vehicle = new operator
    • Frequently share same prefix
    • Hard to be completely avoided. We can merge the classes or use the Intelligent children pattern
  • Inconsistent abstraction level
    • E.g. code in a method should be one level of abstraction below the method’s name
  • Conditional complexity
    • Cyclomatic complexity
      • number of unique way that the code can be evaluated
    • Symptoms: deep nesting (arrow code) & bug ifs
    • Solutions: extract method, strategy pattern, state pattern, decorator
  • Poorly written tests
    • Badly written tests can prevent change
    • Tight coupling

Code Smells: Dispensables

  • Lazy class
    • Classes that don’t do enough to justify their existence should be removed
    • Every class costs something to be understand and maintained
  • Data class
    • Some classes with only fields and properties
    • Missing validation? Class logic split into other classes?
    • Solution: move related logic into the class
  • Duplicated code
    • Violates the DRY principle
    • Result of copy-pasted code
    • Solutions: extract method, extract class, pull-up method, template method pattern
  • Dead code (code that is never used)
    • Usually detected by static analysis tools
  • Speculative generality
    • “Some day we might need…”
    • YAGNI principle

Code Smells: The Couplers

  • Feature envy
    • Method that seems more interested in a class other than the one it actually is in
    • Keep together things that change together
  • Inappropriate intimacy
    • Classes that know too much about one another
    • Smells: inheritance, bidirectional relationships
    • Solutions: move method/field, extract class, change bidirectional to unidirectional association, replace inheritance with delegation
  • The Law of Demeter (LoD)
    • Principle of least knowledge
    • A given object should assume as little as possible about the structure or properties of anything else
    • Bad e.g.: customer.Wallet.RemoveMoney()
  • Indecent exposure
    • Some classes or members are public but shouldn’t be
    • Violates encapsulation
    • Can lead to inappropriate intimacy
  • Message chains
    • Somemthing.another.someother.other.another
    • Tight coupling between client and the structure of the navigation
  • Middle man
    • Sometimes delegation goes too far
    • Sometimes we can remove it or inline it
  • Tramp data
    • Pass data only because something else need it
    • Solutions: Remove middle man, extract class
  • Hidden temporal coupling
    • Occurs when members of a class requires clients to invoke one member before the other
      • Operations consecutively should not be guessed
    • E.g. The use of Pizza class should not know the steps of making pizza
      • Builder or Template Method pattern
  • Hidden dependencies
    • Classes should declare their dependencies in their constructor
    • “new” is glue / Dependency Inversion principle

Data Level Refactorings

  • Replace a magic number with a named constant
  • Rename a variable with more informative name
  • Replace an expression with a method
    • To simplify it or avoid code duplication
  • Move an expression inline
  • Introduce an intermediate variable
    • Introduce explanatory variable
  • Convert a multi-use variable to a multiple single-use variables
    • Create separate variable for each usage
  • Create a local variable for local purposes rather than a parameter
  • Convert a data primitive to a class
    • Additional behavior / validation logic (money)
  • Convert a set of type codes (constants) to enum
  • Convert a set of type codes to a class with subclasses with different behavior
  • Change an array to an object
    • When you use an array with different types in it
  • Encapsulate a collection (list of cards => deck)

Statement Level Refactorings

  • Decompose a boolean expression
  • Move a complex boolean expression into a well-named boolean function
  • Consolidate duplicated code in conditionals
  • Return as soon as you know the answer instead of assigning a return value
  • Use break or return instead ofa loop control variable
  • Replace conditionals with polymorphism
  • Use null objects instead of testing for nulls

Method Level Refactorings

  • Extract method / Inline method
  • Rename method
  • Convert a long routine to a class
  • Add / remove parameter
  • Combine similar methods byparameterizing them
  • Substitute a complex algorithm with simpler
  • Separate methods whose behavior depends on parameters passed in (create new ones)
  • Pass a whole object rather than specific fields
  • Return interface types / base class

Class Level Refactorings

  • Change structure toclass and vice versa
  • Pull members up / pushmembers down the hierarchy
  • Extract specialized code into a subclass
  • Combine similar code into a superclass
  • Collapse hierarchy
  • Replace inheritance with delegation
    • Replace “is-a” with “has-a” relationship
  • Replace delegation with inheritance

Class Interface Refactorings

  • Extract interface(s) / Keep interface segregation
  • Move a method to another class
  • Convert a class to two
  • Delete a class
  • Hide a delegating class (A calls B and C when A should call B and B call C)
  • Remove the man in the middle
  • Introduce (use) an extension class
    • When we don’t have access to the original class
    • Alternatively use decorator pattern
  • Encapsulate an exposed member variable
    • In C# always use properties
    • Define proper access to getters and setters
      • Remove setters to read-only data
  • Hide data and routines that are not intended to be used outside of the class / hierarchy
    • private -> protected -> internal -> public
  • Use strategy to avoid big class hierarchies
  • Apply other design patterns to solve common class and class hierarchy problems (façade, adapter, etc.)

System Level Refactorings

  • Move class (set of classes) to another namespace / assembly
  • Provide a factory method instead of a simple constructor / Use fluent API
  • Replace error codes with exceptions
  • Extract strings to resource files
  • Use dependency injection
  • Apply architecture patterns