Upgrade to Pro — share decks privately, control downloads, hide ads and more …

Diffing Shotgun Surgery and Divergent Change smells in the two editions of Refactoring

Diffing Shotgun Surgery and Divergent Change smells in the two editions of Refactoring

Download for higher quality.

I find out what the changes are to the smells of Shotgun Surgery and Divergent Change in the 2019 edition of Refactoring and realise that some bits have been removed that I found valuable.

E.g. a section has been removed which IMHO highlights and pithily expresses the duality of the two smells, plus naturally links up with how they complement each other in the Single Responsibility Principle, in which the smells also line up with coupling and cohesion.

Philip Schwarz

January 13, 2019
Tweet

More Decks by Philip Schwarz

Other Decks in Programming

Transcript

  1. Shotgun Surgery Shotgun surgery is similar to divergent change but

    is the opposite. You whiff this when, every time you make a kind of change, you have to make a lot of little changesedits to a lot of different classes. When the changes are all over the place, they are hard to find, and it’s easy to miss an important change. In this case you want to use Move Method (142) and Move Field (146) to put all the changes into a single class. If no current class looks like a good candidate, create one. Often you can use Inline Class (154) to bring a whole bunch of behavior together. You get a small dose of divergent change, but you can easily deal with that. Move Function (198) and Move Field (207) to put all the changes into a single module. If you have a bunch of functions operating on similar data, use Combine Functions into Class (144). If you have functions that are transforming or enriching a data structure, use Combine Functions into Transform (149). Split Phase (154) is often useful here if the common functions can combine their output for a consuming phase of logic. A useful tactic for shotgun surgery is to use inlining refactorings, such as Inline Function (115) or Inline Class (186), to pull together poorly separated logic. You’ll end up with a Long Method or a Large Class, but can then use extractions to break it up into more sensible pieces. Even though we are inordinately fond of small functions and classes in our code, we aren’t afraid of creating something large as an intermediate step to reorganization. Divergent change is one class that suffers many kinds of changes, and shotgun surgery is one change that alters many classes. Either way you want to arrange things so that, ideally, there is a one-to-one link between common changes and classes. Shotgun Surgery Shotgun surgery is similar to divergent change but is the opposite. You whiff this when every time you make a kind of change, you have to make a lot of little changes to a lot of different classes. When the changes are all over the place, they are hard to find, and it’s easy to miss an important change. In this case you want to use Move Method (142) and Move Field (146) to put all the changes into a single class. If no current class looks like a good candidate, create one. Often you can use Inline Class (154) to bring a whole bunch of behavior together. You get a small dose of divergent change, but you can easily deal with that. Divergent change is one class that suffers many kinds of changes, and shotgun surgery is one change that alters many classes. Either way you want to arrange things so that, ideally, there is a one-to-one link between common changes and classes. Refactoring - 2000 edition Refactoring - 2019 edition @philip_schwarz Why remove the following? “Divergent change is one class that suffers many kinds of changes, and shotgun surgery is one change that alters many classes. Either way you want to arrange things so that, ideally, there is a one-to-one link between common changes and classes.”
  2. Divergent Change We structure our software to make change easier;

    after all, software is meant to be soft. When we make a change, we want to be able to jump to a single clear point in the system and make the change. When you can’t do this, you are smelling one of two closely related pungencies. Divergent change occurs when one class module is commonly often changed in different ways for different reasons. If you look at a class module and say, “Well, I will have to change these three methods functions every time I get a new database; I have to change these four methods functions every time there is a new financial instrument,” you likely have a situation in which two objects are better than one. That way each object is changed only as a result of one kind of change. this is an indication of divergent change. The database interaction and financial processing problems are separate contexts, and we can make our programming life better by moving such contexts into separate modules. That way, when we have a change to one context, we only have to understand that one context and ignore the other. We always found this to be important, but now, with our brains shrinking with age, it becomes all the more imperative. Of course, you often discover this only after you’ve added a few databases or financial instruments; Any change to handle a variation should change a single class, and all the typing in the new class should express the variation. To clean this up you identify everything that changes for a particular cause and use Extract Class (149) to put them all together. ; context boundaries are usually unclear in the early days of a program and continue to shift as a software system’s capabilities change. If the two aspects naturally form a sequence—for example, you get data from the database and then apply your financial processing on it—then Split Phase(154) separates the two with a clear data structure between them. If there’s more back-and-forth in the calls, then create appropriate modules and use Move Function (198) to divide the processing up. If functions mix the two types of processing within themselves, use Extract Function (106) to separate them before moving. If the modules are classes, then Extract Class (182) helps formalize how to do the split. Divergent Change We structure our software to make change easier; after all, software is meant to be soft. When we make a change we want to be able to jump to a single clear point in the system and make the change. When you can’t do this you are smelling one of two closely related pungencies. Divergent change occurs when one class is commonly changed in different ways for different reasons. If you look at a class and say, “Well, I will have to change these three methods every time I get a new database; I have to change these four methods every time there is a new financial instrument,” you likely have a situation in which two objects are better than one. That way each object is changed only as a result of one kind of change. Of course, you often discover this only after you’ve added a few databases or financial instruments. Any change to handle a variation should change a single class, and all the typing in the new class should express the variation. To clean this up you identify everything that changes for a particular cause and use Extract Class (149) to put them all together. @philip_schwarz Why remove the following? “you likely have a situation in which two objects are better than one. That way each object is changed only as a result of one kind of change.” “Any change to handle a variation should change a single class, and all the typing in the new class should express the variation.” Refactoring - 2000 edition Refactoring - 2019 edition
  3. @philip_schwarz Uncle Bob: as you know, in Refactoring, Martin Fowler

    defines the code smells of Divergent Change (one class that suffers many kinds of changes) and Shotgun Surgery (one change that alters many classes), and says that in order to make it possible, when we make a change, to be able to jump to a single point in the system and make the change, we want to avoid both smells, i.e. we want to arrange things so that ideally, there is a one-to-one link between common changes and classes. If we take your definition of the Single Responsibility Principle (SRP) in ASDPPP (Agile Software Development – Principles, Patterns and Practices) A class should have only one reason to change and we consider Fowler’s statement that Divergent Change occurs when one class is commonly changed in different ways for different reasons. it seems plausible that SRP and Divergent Change are closely related, i.e.: 1. violations of the SRP lead to the smell of Divergent Change 2. Divergent Change is the smell resulting from violations of the SRP More evidence of this relation seems to be provided by Jeff Langr in Clean Code: although he refers readers to the full definition of SRP in ASDPPP, he seems (to me) to concentrate on, and therefore emphasize, the utilisation of SRP as a means to achieve Fowler’s one-to-one link (to ease maintenance): Every sizeable system will contain a large amount of logic and complexity. The primary goal in managing such complexity is to organise it so that a developer knows where to look to find things and need only understand the directly affected complexity at any given time… We want our systems to be composed of many small classes, not a few large ones. Each small class encapsulates a single responsibility, has a single reason to change… On the other hand, in your chapter on the SRP in ASDPPP, I don’t see this emphasis on using the SRP to achieve Fowler’s one-to-one link to ease maintenance. The emphasis seems to be on using the SRP to avoid the risk of orthogonal responsibilites becoming unnecessarily coupled and resulting in fragile code: If a class has more than one responsibility, then the responsibilities become coupled. Changes to one responsibility may impair or inhibit the ability of the class to meet the others. This kind of coupling leads to fragile designs that break in unexpected ways when changed. But if I remember correctly, in some recent podcasts in which you have discussed the SRP, you have said that one of the SRP’s benefits is that the resulting classes are small and have intention-revealing names that make it easy to quickly track down the single class where a change needs to be made. Uncle Bob: how does the SRP relate to Divergent Change? Is the SRP more about using Separation of Concerns to avoid fragility, or is it more about using it to make it as quick as possible to identify the single place where a change is required, or is it about both? Philip: SRP says to keep together things that change for the same reason, and seperate things that change for different reasons. Divergent change occurs when you group together things that change for different reasons. Shotgun surgery happens when you keep apart those things that change for the same reason. So, SRP is about both Divergent Change and Shotgun Surgery. Failure to follow SRP leads to both symptoms. @unclebobmartin https://web.archive.org/web/20090309080035/http://blog.objectmentor.com/articles/2009/02/12/getting-a-solid-start 17/02/2009 2008 2002 2000 Robert Martin
  4. @philip_schwarz Why remove the following? “Divergent change is one class

    that suffers many kinds of changes, and shotgun surgery is one change that alters many classes. Either way you want to arrange things so that, ideally, there is a one-to-one link between common changes and classes.” 7 May 2014 – in a comment on Hacker News article “I don't love the single responsibility principle” https://news.ycombinator.com/item?id=7707189 Philip: SRP says to keep together things that change for the same reason, and separate things that change for different reasons. Divergent change occurs when you group together things that change for different reasons. Shotgun surgery happens when you keep apart those things that change for the same reason. So, SRP is about both Divergent Change and Shotgun Surgery. Failure to follow SRP leads to both symptoms. @unclebobmartin Shotgun Surgery is one change that alters many classes Divergent change is one class that suffers many kinds of changes Shotgun Surgery happens when you keep apart those things that change for the same reason Divergent Change happens when you group together things that change for different reasons keep together things that change for the same reason separate things that change for different reasons Single Responsibility Principle enhancing the cohesion of things that change for the same reasons decreasing the coupling between things that change for different reasons is about SRP: A class should have only one reason to change. SRP is about both Divergent Change and Shotgun Surgery Philip: SRP is about enhancing the cohesion of things that change for the same reasons, and decreasing the coupling between things that change for different reasons It highlights and pithily expresses the duality of the two smells, plus naturally links up with how they complement each other in the SRP, in which the smells also line up with coupling and cohesion. http://blog.objectmentor.com/articles/2009/02/12/getting-a-solid-start Robert Martin Martin Fowler
  5. @philip_schwarz Why remove the following? “you likely have a situation

    in which two objects are better than one. That way each object is changed only as a result of one kind of change.” “Any change to handle a variation should change a single class, and all the typing in the new class should express the variation.” Any change to handle a variation should change a single class i.e. we want to avoid Shotgun Surgery all the typing in the new class should express the variation i.e. it shouldn’t express any other variation, i.e. another reason for change each object is changed only as a result of one kind of change i.e. each object has only one reason to change two objects are better than one i.e. it is better to apply the SRP All the typing in the new class should express the variation c.f. it should express all of the variation, i.e. all of the reason for change Shotgun Surgery is one change that alters many classes Divergent change is one class that suffers many kinds of changes Shotgun Surgery happens when you keep apart those things that change for the same reason Divergent Change happens when you group together things that change for different reasons keep together things that change for the same reason separate things that change for different reasons Single Responsibility Principle enhancing the cohesion of things that change for the same reasons decreasing the coupling between things that change for different reasons is about SRP: A class should have only one reason to change. SRP is about both Divergent Change and Shotgun Surgery They points it makes also line up nicely with the SRP Martin Fowler
  6. The Single-Responsibility Principle (SRP) This principle was described in the

    work of Tom DeMarco and Meilir Page-Jones. They called it cohesion, which they defined as the functional relatedness of the elements of a module. In this chapter, we modify that meaning a bit and relate cohesion to the forces that cause a module, or a class, to change. … The Single-Responsibility Principle: A class should have only one reason to change … each responsibility is an axis of change. When the requirements change, that change will be manifest through a change in responsibility among the classes. If a class assumes more than one responsibility, that class will have more than one reason to change. If a class has more than one responsibility, the responsibilities become coupled. Changes to one responsibility may impair or inhibit the class’s ability to meet the others. This kind of coupling leads to fragile designs that break in unexpected ways when changed. … In the context of the SRP, we define a responsibility to be a reason for change. If you can think of more than one motive for changing a class, that class has more than one responsibility. @unclebobmartin Robert Martin 2002