Advocating for readability at the cost of maintainability

At this point, I think most of us understand that us developers spend more time reading code than writing it. And this is displayed clearly in practice.

Before updating code or adding features, we will spend a decent amount of code identifying the change points, architecting things in our heads, and only after we've paved a theoretical path for ourselves will we actually start writing the new code. This is assuming a large enough code base or architectural footprint that everything cannot exist in ones head.

Working as an Engineer Manager who still does technical work, I've come across countless code bases, where readability is severely compromised in order to ease theoretical maintainability. And I mean theoretical quite literally.

Take the following piece of Terraform code for example:

locals {
  dns_records = {
    "chris-vereulen.com": {
      name = "chris-vermeulen.com"
      type = "A"
      ttl = 3600
      value = "127.0.0.1"
    },
    "inlaymansterms.io": {
      name = "chris-vermeulen.com"
      type = "A"
      ttl = 3600
      value = "127.0.0.1"
    },
  }
}

resource "cloudflare_record" "record" {
  for_each = local.dns_records
  name     = each.value["name"]
  value    = each.value["value"]
  type     = each.value["type"]
  ttl      = each.value["ttl"]
}

This piece of code makes sense. It uses a loop to create DNS records in a list, so we don't have to define a resource for each individual record. It's easy to add new records and delete old ones. Also, if the underlying structure becomes harder in the future, we can update the resource and it will be effective for all DNS records.

Us developers will often times do this in the name of maintainability. "Hey, look how easily we can add more DNS records. We don't even have to create separate Terraform resources.".

Fair play.

However, it's not more maintainable by any reasonable measure.

1. Terraform documents the schema quite effectively. This does not.

2. If the underlying resources for DNS records changes, it will change for all DNS records, and we have to make non-sensible defaults, and weird bypasses in order for the old configurations to function properly.

resource "cloudflare_record" "record" {
  for_each = local.dns_records
  name     = each.value["name"]
  value    = each.value["value"]
  # For example: Previously we set this to A. We need to support CNAME as well. Otherwise we'd need to add type to every record. 
  type     = coalesce(each.value["type"], "A")
  ttl      = each.value["ttl"]
}

3. Say we use a module for each DNS records because it has additional resources per entry, how do we create new records with different versions of the module for validation ?

module "records" {
  for_each = local.dns_records
  
  # How do we update the version for a subset of records to validate ?
  version  = "1.0.1"
  
  name     = each.value["name"]
  value    = each.value["value"]
  type     = each.value["type"]
  ttl      = each.value["ttl"]
}

4. If a more complex DNS record is necessary with additional configuration, do we do a bunch of rope jumping to allow this ? How do we validate individual records effectively ? Look at all the options here: https://registry.terraform.io/providers/cloudflare/cloudflare/latest/docs/resources/record.
Shall we replicate them all ? Usually it means doing something like this:

resource "cloudflare_record" "record" {
  for_each = local.dns_records
  name     = each.value["name"]
  value    = each.value["value"]
  type     = each.value["type"]
  ttl      = each.value["ttl"]
  
  dynamic "data" {
    content {
      weight = try(each.value["data"]["weight"], null)
      proto = try(each.value["data"]["proto"], null)
      regex= try(each.value["data"]["regex"], null)
      [... 36! other options ...]
    }
  }
}

5. Most likely, you'll be reading these records more often than actually adding new ones. And if you're reading them it's likely you're trying to understand how they are configured. Using the local we've defined above, you'd have to go read the Terraform resource anyway, and compute the final product in your head.

In essence, by trying to optimise for writability, you've made both writability and readability harder.

What you should have done from the start was just use Terraform resources. For each record, it adds 2 lines of code more than your loop would have. One of those lines is a closing brace.

There are some valid reasons to have your own structures and loops though.

Let's cover some of them:

1. You have a complex resource structure, which depends on only a few inputs.

If you are creating VPC peers and subnets and networking components for an EC2 instance for example. Most like you'd like to pass it just a few things, which then creates many resources (IAM AMA RIGHT ?!).

In this case, create a module and give it well documented, small scoped variables (Not everything as an object or array). Terraform displayed these variables well if they are used in this way.

An example for our previous example:

module "record" {
  name = "chris-vermeulen.com"
  type = "A"
  ttl  = 3600
}

2. Don't use a module as a way to reduce the amount of code you have

Lots of people will create a module where they pass an array of stuff into it and it create everything for each item. Don't do this. You lose all versioning and proper documentation, and it just creates a layer of indirection which is unnecessary.

A module should configure 1 thing with many resources. In our case the module should represent only 1 DNS record, not many.

3. Use Terraform resources, until you can't maintain them anymore, then use a module.

Unless you're absolutely sure you know what the module should do, implement your new code as resources until it works and you're happy. Then only move them to a module.

This extends far beyond Terraform by the way.

I've seen the same thing in many areas of software. Here are some examples:

• Having a "status" database table, so it's easy to add new statuses for things. Except we then go and use if ($status == 1) in our code.
  Even if we do add a new status in the table, we would need to update the code in order for it to do anything. if ($status == STATUS_ACTIVE) is much better, no ?
• Helm.