Refactoring CS 247 Module 12 Scott Chen Reference

  • Slides: 34
Download presentation
Refactoring CS 247 Module 12 Scott Chen

Refactoring CS 247 Module 12 Scott Chen

Reference – Not Assigned Readings Ø Martin Fowler et al. , Refactoring: Improving the

Reference – Not Assigned Readings Ø Martin Fowler et al. , Refactoring: Improving the Design of Existing Code, 1999, Additon-Wesley • Read as needed to support the lecture contents

Agenda for Module 12 Ø Introduction - Refactoring Ø “The Bad Smells” • •

Agenda for Module 12 Ø Introduction - Refactoring Ø “The Bad Smells” • • Duplicated Code, Long Method, Large Classes, Long Parameter List Divergent Change, Shotgun Surgery, Feature Envy, Data Clumps, Primitive Obsession Switch Statements, Lazy Class, Speculative Generality, Message Chains, Middle Man Inappropriate Intimacy, Alternative Classes with Different Interfaces, Data Class, Refused Bequest, Comments

Section 1 Introduction – Refactoring

Section 1 Introduction – Refactoring

Introduction to Refactoring Ø What is Refactoring? • Reformatting code for improved maintainability and

Introduction to Refactoring Ø What is Refactoring? • Reformatting code for improved maintainability and expandability WITHOUT affecting required functionalities Ø Refactor – When, How Often, and How Much? • • It’s not a standalone task – it should be done as part of the development activities Refactor when you recognize a warning sign of “Bad Smell” (aka. Antipattern) and know what to do § When you add a function § When you fix a bug § When you do a code review Ø When to STOP Refactoring • Changes too invasive that breaks the mental model deployed by other developers § Uninvitingly forcing collaborators to re-learn the design • Too effort- and time-expensive to retest in order to ensure the nothing is broken § Unless you’ve maintained a good set of Unit Test suite Ø BDP: Refactor earlier in design phase to avoid expensive and risky outcome

Introduction to Refactoring Ø Other reasons to NOT refactor: • • • When it’s

Introduction to Refactoring Ø Other reasons to NOT refactor: • • • When it’s a lot easier to rewrite the code from scratch (but give it some good thoughts before committing) The code is too buggy to be worthwhile – rewrite Deadline pressing – too risky Ø A bit on Agile Development • • Methodologies for embracing constant changes – “Don’t fear it, get good at it” Expect to change your mind approaches often, particularly early in development as your learn more about the domain, the schedule, and the features that turn out to be problematic. Ø The Rule of Three (XP) • • • The 1 st time you’re given a task – Just do it The 2 nd time you code the same idea, Wince and code it up again. The 3 rd time you code the same idea, Refactor! • Do not over-engineer – Timely abstraction, expect and embrace changes.

Antipattern and Refactoring Ø The reference book has a catalogue of • 22 Antipatterns

Antipattern and Refactoring Ø The reference book has a catalogue of • 22 Antipatterns – “Bad Smells” § The coding patterns and approaches that lack expandability and flexibility, and can hinder future code development § In this module, Bad Smells will be indicated in Red Italic • 72 Refactoring Techniques § What to do to fix the antipatterns § In this module, Refactoring Techniques will be indicated in Blue Italic • We will be covering the most commonly-seen antipatterns and their refactoring solutions.

Section 2 Bad Smells I (Duplicated Code, Long Method, Large Classes, Long Parameter List)

Section 2 Bad Smells I (Duplicated Code, Long Method, Large Classes, Long Parameter List)

Bad Smells: Duplicated Code Ø The #1 bad smell Ø Same expression in two

Bad Smells: Duplicated Code Ø The #1 bad smell Ø Same expression in two methods in the same class? • Method Extraction • Make it a private helper function and parameterize it. Ø Same code in two related classes? • Form Template Method • • Push commonalities into the closest mutual base class and parameterize (OR, create a new template class) Use Template DP for variation in subtasks

Bad Smells: Duplicated Code Ø Same code in two unrelated classes? • If they

Bad Smells: Duplicated Code Ø Same code in two unrelated classes? • If they are related…Class Extraction, Methods Pull-Up § Class Extraction – create a new abstract base class § Method Pull-Up – pull the common methods up to the base class • If the code logically belongs to just one class… Method Extraction § And make the other class into a client • If the code can be separated into a functor (a function-centric class, to be introduced in Module 18) § Replace Method with Method Object § Encapsulate the common method into a functor, and include the functor class as complex data member for both classes § Strategy DP: Polymorphic variation of methods-as-objects

Not-so-Bad Smell – Cloning Ø Cloning – A type of beneficial Code Duplication •

Not-so-Bad Smell – Cloning Ø Cloning – A type of beneficial Code Duplication • Forking § Duplicate the entire code, then customize for different hardware, platform, or experimental variations • Templating § Boilerplating, API / library protocols, generalized programming idioms, parameterized code • Customizing § Bug workarounds, replicate and specialize

Bad Smells: Long Method Ø Often a sign of • • • Trying to

Bad Smells: Long Method Ø Often a sign of • • • Trying to do too many things Poorly thought out abstraction and boundaries Antipattern arises from micromanagement Ø BDP: Think thoroughly about all major tasks and how they inter-relate, and modularize them • Method Extraction § Break it up into smaller private helper function within the class • Class Extraction, Replace Data Value with Object § Delegate subtasks to specialized subobjects (“know-best” objects) § i. e. Template DP for method Ø Tip (from the reference) – When you see a comment, make a method • • Comments often indicate the next major step, or non-obvious subroutines that detract from the clarity of the whole routine. Either way, “it’s a good time to break up”

Bad Smells: Large Class Ø Too many different data members and member functions Ø

Bad Smells: Large Class Ø Too many different data members and member functions Ø Solution • Class Extraction, Replace Data Value with Object § Gather up the little pieces into aggregate subparts (complex member classes) • Method Extraction § Delegate methods to the new subparts • You may notice some unnecessary subparts that have been hidden in the forest of lengthy design § Resist the urge to micromanage the subparts. Delegate as much as possible to keep your own class neat and clean. Ø Counter Example: • Library classes often have large, fat interface for the purpose of maximized flexibility

Bad Smells: Long Parameter List Ø Making the method really difficult for clients to

Bad Smells: Long Parameter List Ø Making the method really difficult for clients to understand apply Ø Often a symptom of • Trying to do too much, across to many layers of calls, with too many disparate subparts Ø Vintage Solution • Global Variables (not so preferred in OOD method) Ø OOD Solution • Intermediate scope of variables that would make variables appear global within the scope of interest, yet still is protected from external programs. • Method Extraction § Break into sub-tasks • Preserve Whole Object, Introduce Parameter Object (i. e. struct, data-centric class) § Localize passing of parameters § Gather up parameters into aggregate subparts § Simplify the method interface

Section 3 Bad Smells II (Divergent Change, Shotgun Surgery, Feature Envy, Data Clumps, Primitive

Section 3 Bad Smells II (Divergent Change, Shotgun Surgery, Feature Envy, Data Clumps, Primitive Obsession)

Bad Smells: Divergent Change Ø One class is commonly changed in different ways for

Bad Smells: Divergent Change Ø One class is commonly changed in different ways for different reasons • i. e. the class is trying to do too much + contains many unrelated members / subparts Ø Over time, some classes develop a “God Complex” • • They acquire details / ownership of subparts that belong elsewhere Unrelated elements in the same container is a sign of poor design cohesion Ø Solution • Extract Class • Break it up, reshuffle, reconsider relationships and responsibilities – Often involves UML diagram analysis

Bad Smells: Shotgun Surgery Ø The opposite of Divergent Change • Each time you

Bad Smells: Shotgun Surgery Ø The opposite of Divergent Change • Each time you want to make a single, seemingly coherent change, you have to change lots of classes in little ways Ø Also a classic sign of poor design cohesion • Related elements are not in the same container Ø Solution • Move Methods / Fields • Look to do some gathering, either in a new or within the existing class.

Bad Smells: Feature Envy Ø A methods seems more interested in another class than

Bad Smells: Feature Envy Ø A methods seems more interested in another class than the one it is defined in • e. g. , a method A: : m() calls lots of get/set methods of class B. Ø Solution • Extract Method, Move Methods / Fields • Move m(), or just part of it, into B. Ø Exceptions: • • Visitor / Iterator / Strategy DP, where the whole point is to decouple the data from the algorithm Feature envy is more of an issue when both A and B have interesting / independently meaningful data

Bad Smells: Data Clumps Ø A set of variables that are always seen hanging

Bad Smells: Data Clumps Ø A set of variables that are always seen hanging out together • • e. g. , passed as parameters, changed / accessed at the same time. Often a signal that a coherent subobject is crying to be created for encapsulating these “data buddies” Ø Example • • Title is a class dying to be born Shorten the parameter list Conceptually simplify the class design Tradeoff – Client has to talk to another object void Scene: : set. Title (string title. Text, int title. X, int title. Y, Colour title. Colour); void Scene: : get. Title (string& title. Text, int& title. X, int& title. Y, Colour& title. Colour); Ø Solution • Preserve Whole Object, Class Extraction, Introduce Parameter-Centric Object) • • May create Feature Envy initially Require multiple iterations before fully eliminating the bad smell

Bad Smells: Primitive Obsession Ø All data members of an object are instance of

Bad Smells: Primitive Obsession Ø All data members of an object are instance of primitive types • e. g. , dates, currency, SIN, telephone number, ISBN, special string values Ø Usually, these primitive data members have interesting and non-trivial constraints that can be modelled and controlled using repinvariant. • e. g. , fixed number of digits / chars, check digit validity, special values, etc. Ø Solution • Class Extraction, Introduce Parameter-Centric Object, Replace Data Value with Object • • Create some small classes that can validate and enforce the constraints (repinvariants) Make the system more strongly typed

Section 4 Bad Smells III (Switch Statements, Lazy Class, Speculative Generality, Message Chains, Middle

Section 4 Bad Smells III (Switch Statements, Lazy Class, Speculative Generality, Message Chains, Middle Man)

Bad Smells: Switch Statements double get. Speed(){ switch(_type){ case EUROPEAN: return get. Base. Speed();

Bad Smells: Switch Statements double get. Speed(){ switch(_type){ case EUROPEAN: return get. Base. Speed(); case AFRICAN: return get. Base. Speed() – get. Load. Factor * _num. Coconuts; case NORWEGIAN_BLUE: return (_is. Nailed) ? 0 : get. Base. Speed(_voltage); } } Ø This is a prime example of a lack of understanding in polymorphism and encapsulation. Ø Solution • Replace Conditional with Polymorphism • Replace Type Code with Subclasses

Bad Smells: Lazy Class Ø A class that doesn’t do much that’s different from

Bad Smells: Lazy Class Ø A class that doesn’t do much that’s different from other classes • e. g. , if there are several sibling classes that don’t exhibit polymorphic behavioural differences, then consider just collapsing them back into the parent and add some parameters Ø Often, lazy classes are legacies of ambitious design or a refactoring that gutted the class of its original non-trivial behaviour Ø Solution • Collapse Hierarchy • Inline Classes

Bad Smells: Speculative Generality Ø A placeholder routine / module that was once anticipated

Bad Smells: Speculative Generality Ø A placeholder routine / module that was once anticipated to be needed down the road… • • But later on found not really needed anymore Extra classes, features, and flexibility all add to design complexity (and if unnecessary, then Bad Smell) Ø XP Philosophy • • “As simple as possible, but no simpler” “Rule of Three” Ø Solution • Collapse Hierarchy, Inline Classes, Remove Parameter • Refactoring is an ongoing process – if really needed later, you can always add it back.

Bad Smells: Message Chain Ø Client asks an object which asks a subobject, which

Bad Smells: Message Chain Ø Client asks an object which asks a subobject, which asks another subobject, … • • Multi-layer “drill down” may result in sub-sub-objects being passed back to requesting client Sounds like the client already has an understanding of the structure of the object, even if it is going through appropriate intermediaries. Ø Solution • Hide Delegate • i. e. Incorporate a design change to allow direct access to the nested subpart, bypassing the delegating subobjects.

Bad Smells: Middle Man Ø “All hard problems in software engineering can be solved

Bad Smells: Middle Man Ø “All hard problems in software engineering can be solved by an extra level of indirection” • • OO Design Principles pretty much all boil down to this statement, albeit in quite clever and elegant ways. But there is always a limit to how much abstraction should be introduced. Ø If you notice that many of a class’ methods just turn around and beg services of delegate subobjects, the basic abstraction may be poorly thought out. Ø Solution • Remove Middle Man, Replace Delegation with Inheritance • The existence of an object should be validated by its non-trivial purpose and behaviour. Middle Man is exactly the opposite of this.

Section 5 Bad Smells IV (Inappropriate Intimacy, Alternative Classes with Different Interfaces, Data Class,

Section 5 Bad Smells IV (Inappropriate Intimacy, Alternative Classes with Different Interfaces, Data Class, Refused Bequest, Comments)

Bad Smells: Inappropriate Intimacy Ø Sharing of secrets between classes, particularly outside of the

Bad Smells: Inappropriate Intimacy Ø Sharing of secrets between classes, particularly outside of the holy bounds of inheritance • • e. g. , public variables, indiscriminate definition of get/set methods, C++ friendship, protected data in classes The evil root of Representation Exposure Ø Leads to data coupling, and knowledge of internal structure and implementation decisions • Makes clients brittle – hard to evolve, but easy to break Ø Same code in two related classes? • Method / Field Extraction, Change Bidirectional Association to Unidirectional, Hide Delegate • • • Appropriate use of get/set methods Rethink basic abstraction Merge classes if you discover “true love”

Bad Smells: Alternative Classes with Different Interface Ø Classes / Methods seem to have

Bad Smells: Alternative Classes with Different Interface Ø Classes / Methods seem to have the same or similar abstraction, yet are otherwise unrelated. • • Indication of Haphazard Design NOT against overloading Ø Solution: • [Super] Class Extraction, Move Method/Field, Rename Method) • • • Move classes closer together Find common interface Find common subpart and remove it

Bad Smells: Data Class Ø Class consists of simple data fields + simple accessor

Bad Smells: Data Class Ø Class consists of simple data fields + simple accessor / mutator methods • Usually, you’ll find that clients of this class are using get/set methods just like the micromanagement antipattern, though via a level of (pointless) indirection / abstraction. Ø Reasonable to exist, as long as not abused • Can be used as a starting point of design abstraction § “Data classes are like children. They are OK as a starting point, but to participate as a grownup object, they need to take on some responsibilities eventually. ” • Can also use struct as an alternative Ø Solution (if found data class no longer reasonable) • Method Extraction / Move (Pull-up, Push-down) • • Have a look at usage pattern in the clients Try to abstract some commonalities of usage into methods of the data class, and move some functionalities over.

Bad Smells: Refused Bequest Ø Subclass inherits methods / variables but doesn’t seem to

Bad Smells: Refused Bequest Ø Subclass inherits methods / variables but doesn’t seem to use some of them • In a sense, this might be a good sign § Leave all child classes to have the flexibility to derive their own differences from the common parent class • On the other hand, parents may have features that are used by only some of its children § Particularly when the designs of the child classes have matured. Ø In the reference book, this is defined as “not-so-bad smell” • It really depends on whether the abstraction design guides the client to interact with the subclasses and the parent class the desired way. If not, we need to do something. Ø Solution (if refactoring is deemed needed) • Replace Inheritance with Delegation • Push Down

Bad Smells: Comments Ø XP Philosophy discourages sole reliance on comments for documentation •

Bad Smells: Comments Ø XP Philosophy discourages sole reliance on comments for documentation • • • Instead, make methods short and use long, self-explanatory identifiers So that the code itself is more readable as human language (self-evident coding practices) Comments are best used to document design rationale Ø Long comments are often a sign of opaque, complicated, inscrutable code • • Not in the sense of discouraging commenting But it is an indication of the need for restructuring the code Ø Solution (if refactoring is deemed needed) • Method / Class Extraction • Many other refactoring methods mentioned in this module also apply

Summary

Summary

Summary Ø Antipattern • Typical coding styles that negatively impact the manageability and flexibility

Summary Ø Antipattern • Typical coding styles that negatively impact the manageability and flexibility of the OO design Ø Refactoring • • Methods and procedures to eliminate antipatterns without compromising the required functionalities Considerations of whether to refactor – Rule of Three Ø Some Common Refactoring Methods as Design Alternatives • • • Method or Class Extractions / Merge Primitive Data Members v. s. Data-Centric Object Members Multiple Parameters v. s. Parameter-Centric Objects Conditional Code v. s. Polymorphism Inheritance v. s. Composition / Delegation