Working Effectively with Legacy Code Notes

gistfile1.txt

Preface
"They're writing legacy code" - from a consultant working with a new team
Fixing legacy code is like surgery: first make things better and then use that as a catalyst for change.
This book is not about clean code and not about good design

Question: I'm curious to see if knowledge of how you would architect the system now, from scratch is necessary or helpful in going about refactoring it. Do these techniques need you to have an endpoint in mind?


Chapter 1
It doesn't matter if you are adding a feature or fixing a bug, you are solving a problem.
Teams that try to minimize changes are toxic. No room for refactoring or optimization.
Be surgical and minimally invasive with bug fixes, but also understand how it fits into the whole system.
Fixing a bug by refactoring complicated code into simpler code is the best.
Don't scold good Samaritans.

Chapter 2 - Working with Feedback
You can still be "Edit and Pray"ing with tests if your tests don't cover important cases.
Reasons to prefer unit tests over integration tests: Error Localization, Execution Time, Coverage
Has the requirement that tests not talk to the database changed over time and with Rails vs Java?
 - connecting to a test db is easy.
 - doesn't really add any
 
Chapter 3 - Sensing and Separation
Importance of using mocking and stubbing to test unit functionality

Chapter 4 - The Seam Model
Fakes, Mocks and Stubs are soo much easier in Ruby than they were in C and Java

Chapter 5 - Tools
Refactoring tool in RubyMine always made me nervous, not sure what exactly it was doing. Just a smarter rename I think.
Do the types of refactoring tools that he mentions here even exist anymore?
Code Climate is an excellent tool for pointing out messes, but not the same kind of refactoring tool he seems to be talking about.
CppUnitLite - argh WTF, why didn't my manager let me buy this book 6 years ago!

Chapter 6 - I don't have much time and I have to change it
Changes cluster in a system, you're more likely to change things that you've just changed
Yesware/Zuora stuff was Sprout Method/Class
When using wrap method/class, you can add above the current method or below, decide which fits the system best, but sometimes its much easier to insert below even if that isn't the best for the system.

Chapter 7 - It takes forever to make a change

Chapter 9 - I Can't Get This Class into a Test Harness

The Case of the Irritating Parameter
Step 1: Try to create the object in a test with no assertions (or in Ruby, assert true or expect 1 to equal 1)
  in our example, the CreditValidator
Step 2: Add necessary information that construction complains about not having (the required parameters)
  in our example, the RGHConnection which connects to a server and gets all the reports needed
  and CreditMaster that loads a bunch of files that contain policy information from disk into memory
Step 3: Asses viabilit of what you just did
  connecting to server is a very bad idea in test harness
  loading files from disk he says is ok, but I think you need test files and those are a pain in the ass
Step 4: create fake versions of inconvenient parameters, in our case RGHConnection
  use Extract Interface (normally we'd do this with a stub)
  rules are different for fake classes, its ok to have methods that don't do anything and others that return null or empty bodies
  test code should be clean and clear first, less worried about duplication
Step 5: Pass Null to anything that your test doesn't need. It will blow up somewhere usefull if its really needed

Side note: if you have to fake things out too deep, you know you are violating the Law of Demeter

Null Object Pattern
 - I've never used this unless an empty array counts, has anyone used this with success in Ruby?
 
The Case of the Hidden Dependency
 Accessed by your class but not passed in
  - can pass it in with Parameterize Contructor technique, also known as dependency injection
  - for things like mailers, there is often a useful global way to handle in tests

The Case of the Construction Blob
 Constructor creates a large amount of objects either itself or by cascading construction
 Parameterize Constructor is a bad idea
 Supercede Instance Variable: define your own getter and setter
  - resource management not an issue in Ruby, but still not a great idea
 Extract and Override Factory Method is pretty straight forward but then you have to rebuild your own constructor?
 My take away from this section: all options are bad, but you need to do something before changing too much.

The Case of the Irritating Global Dependency
 usually Singletons - good pattern if you need it, still not ideal to have around
 hidden dependencies
 can make code much more difficult to understand
 code on page 121 is basically an ||= (Introduce Static Setter Pattern)
 decide whether this thing really ought to be a singleton of not
 Singletons are also a legitimately good use of fixture data
 
The Case of the Horrible Include Dependency
 this part is mostly about C++, but it is a good reminder that extracting code from a class into a module and including it, doesn't really make it a separate object.
 also coupling is bad in all of its forms, even if your language doesn't make it quite so painful

The Case of the Onion Parameter
 object must create the entire world to live in
 particularly problematic in Rails with validates presence of
 FactoryGirl can hide the issue
 only way to solve it is to break dependencies

The Case of the Aliased Parameter
 seems like the same sort of thing as the Onion Parameter