DEV Community

loading...
Cover image for Refactoring: Change Working Code?

Refactoring: Change Working Code?

jwp profile image John Peters Updated on ・2 min read

Assume we have a "Thing" type, with a constructor that already works, it is considered "Done" in Agile. A new change comes in whereby another field needs to be added. Do we change the constructor to include the new field?

Answer: Maybe, but we don't have to do that.

/** Creates a new Thing 
* @returns a Thing */
function getThing(
person,
address,
delivery,
account):Thing {
   let thing = new Thing(
      person,
      address,
      delivery,
      account     
   );
 return thing;
}

Enter fullscreen mode Exit fullscreen mode

Thing has been integrated for 2 years into existing production code. A new requirement comes along to create a "Delinquent Delivery Thing" due to a production delay.

Scenario 1
Relies on creating a new Class named Delinquency, which will contain Thing (our already working code). If we were to add a new property to the Thing constructor it would have to be optional to stop compile errors in existing code. This is not optimal.


function getDelinquentThing(
  // Assumes we have a Class named Delinquency
  person,
  address,
  delivery,
  account, 
  delinquency
) : Delinquency {
    let thing = getThing(
  person,
  address,
  delivery,
  account
);
 let containedDelinquency = 
   new Delinquency(
     thing, 
     delinquency
   );    
}
Enter fullscreen mode Exit fullscreen mode

Scenario 2

Appends a runtime property to the Thing class.


function getDelinquentThing(
  // We append the property
  person,
  address,
  delivery,
  account, 
  delinquency
){
    let thing = getThing(
  person,
  address,
  delivery,
  account);
    thing.delinquency = delinquency


}
Enter fullscreen mode Exit fullscreen mode

Scenario 2 has future liabilities. There's a property attached to a Type that is not discoverable. This means that developers, down the road only know about the Thing type. For experienced developers, this is not a problem, for new folks this can be a major tripping point. Finding properties in JavaScript is not a problem, it's just knowing when to look for them.

Don't Change Working Code

Example of a change to working code.

class Thing{
 person:Person;
 address:Address;
 delivery:Delivery;
 account:Account;
 // This is a change to working code
 delinquency: Delinquency;
}
Enter fullscreen mode Exit fullscreen mode

Many will say; this is not a problem to do this. Indeed it's typically not an issue, due to separation of concerns, we simply added a new property in the Thing object. Code that doesn't know about the new property will continue to work as it did.

This is only true after we fix all the compile errors, which forces a lot of changes to working areas that may not care about delinquency at all.

In keeping with "Open/Closed" principle, we should favor Scenario 1.

JWP2020 Don't Change Working Code

Discussion

pic
Editor guide