Swizec Teller - a geek with a hatswizec.com

Senior Mindset Book

Get promoted, earn a bigger salary, work for top companies

Senior Engineer Mindset cover
Learn more

    Bad excuses to want a rewrite

    Here's a sneak peek at a new book I'm writing about Refactoring and Rewriting. It's with a publisher so I might get in trouble for sharing this, but it's a first draft ... they own the final version 😛

    This code is terrible. We should rewrite.

    Literally every new engineer on every team.

    1. Does it work?
    2. Has the context changed?

    Okay then leave it alone. It’s fine.

    Every one of those little things you see that looks weird is a bug fix. A lesson learned. An edge case found. A painful memory enshrined in code.

    You start with a simple function that responds to an event and updates the database.

    async function handleEvent(event) {
      const item = await readFromDB(event.item.id)
    
      if (event.newState) {
        await item.save({
          status: event.newState,
        })
        await notifyUserOfChange(item)
      }
    
      return `Status updated to ${event.newState}`
    }
    

    Pretend this function is called from a queue or notification service. You get an event that happened on event.item and update the item.status field in the database. After saving, you send a notification to the owner of this item. Like an email that says ”Your item is out for delivery”

    One day the notification service sends an event with no item. Due to a weird bug in readFromDB, your database locked up for 5 minutes and caused a huge outage.

    Both the notification service and the database are outside your control. Best you can do is to check before reading.

    async function handleEvent(event) {
      if (!event.item) {
        throw new Error(`Missing event.item`)
      }
    
      const item = await readFromDB(event.item.id)
    
      if (event.newState) {
        await item.save({
          status: event.newState,
        })
        await notifyUserOfChange(item)
      }
    
      return `Status updated to ${event.newState}`
    }
    

    You now throw an error when there’s no item in the event. The error handling framework will handle logging and sending the right response status to the notification service.

    A few days pass and you notice something strange in the logs. There’s a lot of database errors when calling item.save. 🤨 You can barely find the logs you care about through the stack traces and convoluted error messages.

    After some digging you find that the notification service is pinging you for items that don’t exist. You can’t fix that even if you wanted to. You can add another check to your code though.

    async function handleEvent(event) {
      if (!event.item) {
        throw new Error(`Missing event.item`)
      }
    
      const item = await readFromDB(event.item.id)
    
      if (!item) {
        throw new Error(`Item not found`)
      }
    
      if (event.newState) {
        await item.save({
          status: event.newState,
        })
        await notifyUserOfChange(item)
      }
    
      return `Status updated to ${event.newState}`
    }
    

    Great. You make sure the item was found in your database before trying to do anything else. Still an error, but a correctly handled one.

    A month passes. Your function is doing great.

    ”Hey [name|] we had a user complain they got 500 emails about their item being out for delivery. They’re pretty pissed”

    That’s odd.

    The notification service must have got stuck in a loop and sent the same event 500 times. Or there was an error in notifyUserOfChange after the email goes out, but before we tell the notification service the event was handled. Kept retrying 💩

    You add another check. This time looking for idempotency – calling the function with the same arguments creates the same result.

    async function handleEvent(event) {
      if (!event.item) {
        throw new Error(`Missing event.item`)
      }
    
      const item = await readFromDB(event.item.id)
    
      if (!item) {
        throw new Error(`Item not found`)
      }
    
      if (event.newState && event.newState !== item.status) {
        await item.save({
          status: event.newState,
        })
        await notifyUserOfChange(item)
      }
    
      return `Status updated to ${event.newState}`
    }
    

    Fantastic. Now the status update only happens, if newState is different than existing status. No more notification spam 🙌

    Until a user says ”Hey I got an out for delivery email 2 days after the item was delivered”

    You check the database and sure enough, the item is marked as “out for delivery” even though you can dig up a log that says it was marked as “delivered” 2 days before. Your database is wrong!

    Again, the notification service had a hiccup and sent events out of sequence. You need to make sure transitions are valid before updating your state. Also known as the actor model of computation.

    async function handleEvent(event) {
      if (!event.item) {
        throw new Error(`Missing event.item`)
      }
    
      const item = await readFromDB(event.item.id)
    
      if (!item) {
        throw new Error(`Item not found`)
      }
    
      if (isValidTransition(event, item)) {
        if (event.newState && event.newState !== item.status) {
          await item.save({
            status: event.newState,
          })
          await notifyUserOfChange(item)
        }
      }
    
      return `Status updated to ${event.newState}`
    }
    

    Wow look at that code. It’s so ugly – we should rewrite.

    Cheers, ~Swizec

    PS: you can share this if you really want me to get into trouble

    Published on April 28th, 2022 in Uncategorized

    Did you enjoy this article?

    Continue reading about Bad excuses to want a rewrite

    Semantically similar articles hand-picked by GPT-4

    Senior Mindset Book

    Get promoted, earn a bigger salary, work for top companies

    Learn more

    Have a burning question that you think I can answer? Hit me up on twitter and I'll do my best.

    Who am I and who do I help? I'm Swizec Teller and I turn coders into engineers with "Raw and honest from the heart!" writing. No bullshit. Real insights into the career and skills of a modern software engineer.

    Want to become a true senior engineer? Take ownership, have autonomy, and be a force multiplier on your team. The Senior Engineer Mindset ebook can help 👉 swizec.com/senior-mindset. These are the shifts in mindset that unlocked my career.

    Curious about Serverless and the modern backend? Check out Serverless Handbook, for frontend engineers 👉 ServerlessHandbook.dev

    Want to Stop copy pasting D3 examples and create data visualizations of your own? Learn how to build scalable dataviz React components your whole team can understand with React for Data Visualization

    Want to get my best emails on JavaScript, React, Serverless, Fullstack Web, or Indie Hacking? Check out swizec.com/collections

    Did someone amazing share this letter with you? Wonderful! You can sign up for my weekly letters for software engineers on their path to greatness, here: swizec.com/blog

    Want to brush up on your modern JavaScript syntax? Check out my interactive cheatsheet: es6cheatsheet.com

    By the way, just in case no one has told you it yet today: I love and appreciate you for who you are ❤️

    Created by Swizec with ❤️