Video: High-Quality Code - Refactoring: Improving the Quality of Existing Code (Bulgarian)

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