Discussion on: How do you know when to NOT refactor?

View post

Replies for: tl;dr – I guess the inverse of all of this is, "When you can read through a block of code, and understand what it does on your first read-through o...

First off, wow, the depth here is fantastic. Thanks so much for taking the time.

I'm struggling to follow your recommended refactor for #1 and #2. Maybe I'm hung on the term "polymorphic dispatch" (no matter how many times I read descriptions of polymorphism, the meaning doesn't stick to the word for me).

Here's an example of what I think you mean by #2:

function SubmitContactRequest(ContactRequest request, RequestType type) {
    DBConnection dbConn = GetDBConnection("contactdb");

    if (type == RequestType.Phone) {
        dbConn.run("NewPhoneContactRequest", request);
    } else if (type == RequestType.Email) {
        dbConn.run("NewEmailContactRequest", request);
    }
}

If that is an accurate example, how would you refactor with polymorphic dispatch?

Again, thanks so much. I'm honestly daunted trying to learn when to avoid refactoring, and this is super helpful.

Josh • Jul 10 '19 • Edited

I'm glad I could help! Refactors used to be my least favorite aspect of writing code, but once a mentor demonstrated how important they are to having maintainable, updateable code, keeping my own codebases tidy became a matter of respect to him, and pride to myself 😅

To your question: Yes, that's exactly the sort of scenario I was thinking of! I haven't used TypeScript before, so someone else will have to let you know if this compiles, but I essentially mean that you would give two methods on different classes the same signature, like so:

class PhoneRequest extends ContactRequest {
  // PhoneRequest wouldn't necessarily have to extend 
  // ContactRequest in JavaScript, but idk how it works
  // in this Type-Scripted world 😬

  // ...
  handleSubmit(DBConnection conn) { 
    /* process submission in a phoney manner */ 
  }  
}

class EmailRequest extends ContactRequest {
  // ...
  handleSubmit(DBConnection conn) { 
    /*  process submission in an emailey way */
  }
}

That will allow you to remove the entire if-else if-else tree that pertains to handling the check against the passed-in type

function SubmitContactRequest(ContactRequest contactReq) {
  const DBConnection conn = GetDBConnection("contactdb");
  contactReq.handleSubmit(conn);
}

Now, no matter how many types of contact request you submit through this function, adding support for a new type is an O(1) developer problem, because you can guarantee that adding e.g. FaxRequest#handleSubmit will not ever interfere with the behavior of either PhoneRequest or EmailRequest's own #handleSubmit method.

Let me know if you have any other questions!

Michael Crenshaw • Jul 10 '19

No worries about the syntax. I kinda invented a C#-TypeScript hybrid there. :-)

That's really helpful! I like your description of developer-complexity as O(1). I'll probably return to this example when looking at future refactors. Thanks again!