Not all accusations are baseless

They don’t work anymore, those baseless accusations that anyone we disagree with is a racist, misogynist, fascist.

OK, so, DHH wrote this. The post, I assume (see later) largely written to say “look, I was right about DEI”, also pretty clearly suggests that applying those labels to Trump is “baseless”, and that the election demonstrated that people now see through a noisy minority throwing those accusations – at Trump in this case – for the overall good of everyone. The wisdom of the crowd has overcome political correctness, maybe.

Erk.

The problem with accusations like that is that they eventually have to be backed by proof or they’ll bounce like a rubber check. And when even the most mundane political or moral positions were able to earn one of those *ist-y labels, the entire enterprise of throwing them around was bound to go bankrupt. And now it has.

It’s impossible to consider Trump’s political or moral positions as mundane, so given this post is actually about the election it’s very unclear to me why we’d be talking about mundane positions. When we talk about Trump, nothing is mundane or even moderate. Not even his supporters would describe him as that.

That big scary mob that once was has been reduced to near impotence in most arenas, including the biggest of all, the US presidential election. That’s a win for everyone whether they like Donald Trump or not.

If I had to summarise David’s post, it’d be something like: “all the people who voted for Trump demonstrated that throwing around terms like ‘racist’ and ‘fascist’ doesn’t work to scare people into behaving a certain way, and that’s a good thing.”

Hopefully even he would agree that was a fair summary.

But what if the accusations aren’t baseless?

His ex Chief of Staff thinks he fits the general description of a fascist. His disrespect for the democratic process is clearly authoritarian. He has stated plainly that he wants to silence or remove his opposition.

He fits the definition. Calling him a fascist is hardly baseless, unless you’re going to argue “oh, he’s just saying that, he doesn’t actually believe it or won’t actually do it.” I don’t buy that.

His anti-immigrant claims that immigrants are eating pets are racist. So many other instances of racism are well documented.

You might be able to argue that he’s never been technically convicted of rape, but a jury found him liable for battery through sexual assault. The evidence of his disrespect for women is so easy to find, and so irrefutable, that I won’t even both linking to it.

So these accusations, applied to Trump, are about as far from baseless as you can get.

What, then, can we learn from the election result about baseless accusations then?

Not much, I’d say.

But what if Trump’s win actually demonstrates, terrifyingly, that people don’t care even that he demonstrates racist and fascist behaviours? That they don’t care he’s a conviced criminal, that he’s been indicted four times, that he has no regard for the law. What if the election shows that none of that matters, and that people will vote for him anyway, no matter how corrupt and craven he fucking demonstrably is?

What if the “bankrupcy” is actually the majority’s willingness to overlook objective facts and instead wallow in an individualist fantasy being served up by a wannabe strongman feeding them bullshit stories about how their problems are all somebody else’s fault, and how he’s going to somehow restore them to a “greatness” that only ever existed in a fairy tale? How is that a win for everyone?

OK, look, sorry for the swear. Maybe I’m reading more into what David has written than he intended. I’ve read what he wrote a bunch of times and I’ve tried to find the generous read. I’ve actively worked to try to understand the point he’s trying to make.

I have introspected enough to know that it’s human nature to view events through a lens that reinforces and supports our existing opinions, so I’m not surprised that the election makes David think about DEI. And he can, of course, say whatever he wants, and I respect his right to do that.

And I genuinely appreciate what he contributes, apolitically, to the community I love, even if I disagree with where some of his non-development trains of thought have led him.

But, IMHO, this post seems to be, at best, a pointless and stretched-thin linking of DEI to “woke mob accusations” to the election of a singularly, objectively not good person into a position of incredible power.

And at worst: deliberately suggesting that Trump’s win is “good for everyone” because it demonstrates that calling anyone a fascist or a racist or a sexist or a bigot or a misogynist no longer has any impact on their ability to progress. No matter how accurate those labels actually are.

You might be able to keep politics out of your workplace. But you can’t keep politics out of politics.

Not everyone I disagree with is a racist, misogynist fascist.

But that doesn’t mean Donald Trump isn’t.

Because he very, very obviously is.

Conversations are hard

Sometimes I find talking to people very hard. Even people I like a lot. And I’ve been thinking about why.

This (ugh) Quora answer actually summarises it pretty well:

Question askers navigate conversations by asking questions, getting responses, asking more questions, and hoping that they will get a question in return so that they can talk about themselves a little too. They will listen to openly shared stories, but they won’t openly share their own stories.

Open sharers will choose topics about their own lives to share, and hope that the other person will do the same. They will answer questions, but won’t ask them.

What I’ve noticed is that, while I can do “open sharing”, I am much more often a “question asker”. I listen attentively to your stories and I ask questions to help you talk more about whatever is interesting you at the moment.

But what I’ve also noticed, is that it really sucks sometimes when I’m talking to an open sharer, even if I really like the person, when they never switch modes and ask any questions1. In those conversations, it feels like the onus is always on me to switch modes and volunteer some hopefully-interesting thought or anecdote if I want to feel like I’m anything other than my conversational partner’s therapist. Doing that time after time eventually feels like I’m pushing for attention, which is icky and exhausting. I would like to feel like I am interesting to you without having to beg for your attention.

I don’t think that open sharers are bad people; it’s just how they’ve learned to interact, and when they talk with other open sharers then it works totally fine for them. But sometimes it feels like they are oblivious to the fact that not everyone is the same as them. They might not even realise they are an open sharer, since their mode is by default the one that “creates” conversation.

Think about what kind of conversationalist you are. Really think about it. In your last conversation with your good friend, did you ask them any questions? Have a scroll up. When was the last time you asked them anything meaningful at all? Has it been a while? Guess what, buddy: you might be an open sharer.

My suggestion to all you open sharers out there, if you actually care about how the person you are talking to feels: pay attention in your conversations, and if it feels like your partner hasn’t said much, then maybe ask them a fucking question or two.

  1. OK, to be fair, conversations with an extreme question asker can also be pretty unpleasant; even for another question asker they can feel like an interrogation. 

Weeknotes 2041

Work

A lot of my work at the moment involves chasing down bugs in an incredibly complicated publishing system which has multiple triggers (both internal and external) that make films available to watch on a variety of platforms at specific times in specific countries. When bugs creep in, films either stay up or don’t go up, and sometimes that annoys the wrong people. This week was one of those weeks, but involving a bug that was introduced about two years ago, and then fixed 8 months later, so calculating the true impact of the issue is pretty difficult. This is definitely why I got into software development.

Not work

Does this site now run on Ruby 3.1? I hope it does. I’ve been slowly working through a bunch of my other small applications, trying to update them to modern versions of Ruby (and even Rails), so that at the very least they remain deployable. This site is one of those small applications.

My real job

As the parent of an infant, I am now accustomed to exhaustion, and within that broad label, all the subtly-distinct flavours it comes in. This week has mostly been standard not-enough-contiguous-hours-of-unconsciouness tiredness. Number of thousand-yard stares while the child does not immediately need my attention: 5.

Physical health

None of this is helped by having yet another cold of some kind. Which of course we all get, and so it’s a never ending cycle of minor illness and sleep distruption and nobody is feeling very jazzy or sparkly but at the same time, the kid is so damned adorable that it remains worth it.

Mental health

Some rare adult socialisation in the form of a birthday party for another child (but yes, it still counts), and then Tom & Nat coming over for lunch today, both of which were very excellent.

Door

Today I found a door in my hallway that I hadn’t noticed before, but I’m currently too tired to investigate further.

Connoisseur

Listen, here’s what I’m wondering: is it better to be able to appreciate subtle refinements and improvements of a thing, the finest examples of it, if it comes at the expense of enjoying the version of that thing that’s most commonly or easily available?

If that “better” thing gives an increase of happiness of X, but the more commonly available instances – in their relative worseness – now gives a decrease of happiness of Y, then how often is the sum of X greater than the sum of Y?

Is it better to really get into, say, wine, for all the increased enjoyment that those few really great wines can provide, if it means that you either need to spend a new multiple on acquiring that wine, or suffer increasing disappointment from the wines you can affort or are offered at friends houses?

Or is it better – on the whole – to remain largely ignorant of what distinguishes a “great” wine from an “ok” wine, and just enjoy them all to a lesser but more consistently positive extent?

It might not be wine, it could be chocolate, or coffee, or art, or pretty much anything else that you can develop a “taste” for. Are the newly-available highs of pleasure worth, overall, the lessened ability to tolerate the instances of that thing which do not match your new palate?

This is what I’m wondering. Sound out in the comments1

The only clue I can offer is that it’s a literal nightmare figuring out how to spell “connoisseur”. I had to look it up to write this. Twice.

The universe weaves clues into its ineffable fabric, I suppose.

  1. JK there are no comments here – literal LOL – nah, you go scream into whatever digital silo you prefer. 

This is for Patrick

This is for Patrick, who doesn’t subscribe to the RSS or Atom feeds.

Instead he sets a reminder to check this website every week or two (or perhaps less, I forget), and – upon seeing no new posts – sets another reminder to do the same later that month, or year, or whenever.

This is for Patrick, who has lost so many precious seconds checking this site for new posts, only to find nothing.

I won’t be responsible for this anymore. I won’t burn his time and energy like electron potential on the bitcoin bonfire. I won’t be the source of that disappointment. My stale blog-type thing won’t be the reflection in that lone tear as it traces its melancholy path down his face. Not anymore.

This post, Patrick… buddy… this is for you.

Don't cache ActiveRecord objects

Hello, new Rails developer! Welcome to WidgetCorp. I hope you enjoyed orientation. Those old corporate videos are a hoot, right? Still, you gotta sit through it, we all did. But anyway, let’s get stuck in.

So as you know, here at WidgetCorp, we are the manufacturer, distributor and direct seller of the worlds highest quality Widgets. None finer!

These widgets are high-value items, it’s a great market to be in, but they’re also pretty complicated, and need to be assembled from any number of different combinations of doo-hickeys, thingamabobs and whatsits, and unless we have the right quantities and types of each, then a particular kind of widget may not actually be available for sale.

(If you just came for the advice, you can skip this nonsense.)

But those are manufacturing details, and you’re a Rails developer, so all it really means for us is this: figuring out the set of available widgets involves some necessarily-slow querying and calculations, and there’s no way around it.

In our typical Rails app, here’s the code we have relating to showing availble widgets:

# db/schema.rb
create_table :widgets do |t|
  t.string :title
  # ...
end

# app/models/widget.rb
class Widget < ActiveRecord::Base
  scope :available, -> { 
    # some logic that takes a while to run
  }
end

# app/helpers/widget_helper.rb
module WidgetHelper
  def available_widgets
    Widget.available
  end
end
<!-- app/views/widgets/index.html.erb -->
<% available_widgets.each do |widget| %>
  <h2><%= widget.name %></h2>
  <%= link_to 'Buy this widget', widget_purchase_path(widget) %>
<% end %>

It’s so slow!

The complication of these widgets isn’t our customer’s concern, and we don’t want them to have to wait for even a second before showing them the set of available widgets that might satisfy their widget-buying needs. But the great news is that the widget component factory only runs at midnight, and does all its work instantaneously1, so this set is the same for the whole day.

So what can we do to avoid having to calculate this set of available widgets every time we want to show the listing to the user, or indeed use that set of widgets anywhere else in the app?

You guessed it, you clever Rails developer: we can cache the set of widgets!

For the purposes of this example, let’s add the caching in the helper:2

module WidgetHelper
  def available_widgets
    Rails.cache.fetch("available-widgets", expires_at: Date.tomorrow.beginning_of_day) do
      Widget.available
    end
  end
end

Bazinga! Our website is now faster than a ZipWidget, which – believe me – is one of the fastest widgets that we here at WidgetCorp sell, and that’s saying something.

Anyway, great work, and I’ll see you tomorrow.

The next day

Welcome back! I hope you enjoyed first night in the company dorm! Sure, it’s cosy, but we find that the reduced commute times helps us maximise employee efficiency, and just like they said in those videos, say it with me: “An efficient employee is… a…”

… more impactful component in the overall WidgetCorp P&L, that’s right. But listen to me, wasting precious developer seconds. Let’s get to work.

So corporate have said they want to store the colour of the widget, because it turns out that some of our customers want to coordinate their widgets, uh, aesthetically I suppose. Anyway, I don’t ask questions, but it seems pretty simple, so let’s add ourselves the column to the database and show it on the widget listing:

# db/schema.rb
create_table :widgets do |t|
  t.string :title
  t.string :colour
  # ...
end
<!-- app/views/widgets/index.html.erb -->
<% available_widgets.each do |widget| %>
  <h2><%= widget.title %></h2>
  <p>This is a <%= widget.colour %> widget</p>
  <!-- ...  -->
<% end %>

… aaaaaaaand deploy.

Great! You are a credit to the compa WHOA hangon. The site is down. THE SITE IS DOWN.

Exceptions. Exceptions are pouring in.

“Undefined method colour”? What? We ran the migrations right? I’m sure we did. I saw it happen. Look here, I’m running it in the console, the attribute is there. What’s going on? Oh no, the red phone is ringing. You answer it. No, I’m telling you, you answer it.

What’s going on

The reason we see exceptions after the deployment above is that the ActiveRecord objects in our cache are fully-marshalled ruby objects, and due to the way ActiveRecord dynamically defines attribute access, those objects only know about the columns and attributes of the Widget class at the time they entered the cache.

And so here’s the point of this silly story: never store objects in your cache whose structure may change over time. And in a nutshell, that’s pretty much any non-value object:

Marshalling data

If you take a look at how Rails caching actually works, you can see that under the hood, the data to be cached is passed to Marshal.dump, which turns that data into an encoded string.

We can see what that looks like here:

$ widget = Widget.create(title: 'ZipWidget')
$ data = Marshal.dump(widget)
# \x04\bo:\vWidget\x16:\x10@new_recordF:\x10@attributeso:\x1EActiveModel::AttributeSet\x06;
# \a{\aI\"\aid\x06:\x06ETo:)ActiveModel::Attribute::FromDatabase\n:\n@name@\b:\x1C
# @value_before_type_casti\x06:\n@typeo:\x1FActiveModel::Type::Integer\t:\x0F@precision0
# :\v@scale0:\v@limiti\r:\v@rangeo:\nRange\b:\texclT:\nbeginl-\t\x00\x00\x00\x00\x00\x00\x00\x80:\b
# endl+\t\x00\x00\x00\x00\x00\x00\x00\x80:\x18@original_attribute0:\v@valuei\x06I\"\ntitle\x06;
# \tTo;\n\n;\v@\x0E;\fI\"\x0EZipWidget\x06;\tT;\ro:HActiveRecord::ConnectionAdapters::AbstractMysqlAdapter::MysqlString\b;
# \x0F0;\x100;\x11i\x01\xFF;\x170;\x18I\"\x0EZipWidget\x06;\tT:\x17@association_cache{\x00:\x11
# @primary_keyI\"\aid\x06;\tT:\x0E@readonlyF:\x0F@destroyedF:\x1C@marked_for_destructionF:\x1E
# @destroyed_by_association0:\x1E@_start_transaction_state0:\x17@transaction_state0:\x17
# @inspection_filtero:#ActiveSupport::ParameterFilter\a:\r@filters[\x00:\n@maskU:
# 'ActiveRecord::Core::InspectionMask[\t:\v__v2__[\x00[\x00I\"\x0F[FILTERED]\x06;\tT:$
# @_new_record_before_last_commitT:\x18@validation_context0:\f@errorsU:\x18
# ActiveModel::Errors[\b@\x00{\x00{\x00:\x13@_touch_recordT:\x1D@mutations_from_database0: 
# @mutations_before_last_saveo:*ActiveModel::AttributeMutationTracker\a;\ao;\b\x06;\a{\a
# @\bo:%ActiveModel::Attribute::FromUser\n;\v@\b;\fi\x06;\r@\n;\x17o;\n\n;\v@\b;\f0;\r
# @\n;\x170;\x180;\x18i\x06@\x0Eo;0\n;\v@\x0E;\fI\"\x0EZipWidget\x06;\tT;\r@\x11;\x17o;\n\t;
# \v@\x0E;\f0;\r@\x11;\x170;\x18@\x10:\x14@forced_changeso:\bSet\x06:\n@hash}\x00F

If you look closely in there, you can see some of the values (e.g. the value ZipWidget), but there’s plenty of other information about the specific structure and implementation of the ActiveRecord instance – particularly about the model’s understanding of the database – that’s encoded into that dump.

You can revive the object by using Marshal.load:

$ cached_widget = Marshal.load(data)
# => #<Widget id: 1, title: "ZipWidget">

And that works great, until you try and use any new attributes that might exist in the database. Let’s add the colour column, just using the console for convenience:

$ ActiveRecord::Migration.add_column :widgets, :colour, :string, default: 'beige'

We can check this all works and that our widget in the database gets the right value:

$ Widget.first.colour
# => 'beige'

But what if we try and use that same widget, but from the cache:

$ cached_widget = Marshal.load(data)
# => #<Widget id: 1, title: "ZipWidget">
$ cached_widget.colour
Traceback (most recent call last):
        1: from (irb):14
NoMethodError (undefined method `colour' for #<Widget id: 1, title: "ZipWidget">)

Boom. The cached instance thinks it already knows everything about the schema that’s relevant, so when we try to invoke a method from a schema change, we get an error.

Workarounds

The only ways to fix this are

Not great.

But there’s a better solution, which is avoiding this issue in the first place.

Store IDs, not objects

Sometimes it is useful to take a step back from the code and think about what we are trying to acheive. We want to quickly return the set of available widgets, but calculating that set takes a long time. However, there’s a difference between calculating the set, and loading that set from the database. Once we know which objects are a part of the set, actually loading those records is likely to be pretty fast – databases are pretty good at those kinds of queries.

So we don’t actually need to cache the fully loaded objects; we just need to cache something that lets us quickly load the right objects – their unique ids.

Here’s my proposed fix for WidgetCorp:

module WidgetHelper
  def available_widgets
    ids = Rails.cache.fetch('available-widgets', expires_at: Date.tomorrow.beginning_of_day) do
      Widget.available.pluck(:id)
    end
    Widget.where(ids: ids)
  end
end

Yes, it’s less elegant that neatly wrapping the slow code in a cache block, but the alternative is a world of brittle cache data and deployment fragility and pain. If a new column is added to the widgets table, nothing breaks because we’re not caching anything but the IDs.

Bonus code: defend your app from brittle cache objects

It can be easy to forget this when you’re building new features. This is the kind of thing that only bites in production, and it can happen years after the unfortunate cache entered use.

So the best way of making sure to avoid it, is to have something in your CI build that automatically checks for these kinds of records entering your cache.

In your config/environments/test.rb file, find the line that controls the cache store:

# config/environments/test.rb
config.cache_store = :null

and change it to this:

# config/environments/test.rb
config.cache_store = NullStoreVerifyingAcceptableData.new

And in lib, create this class:

# lib/null_store_verifying_acceptable_data.rb
class NullStoreVerifyingAcceptableData < ActiveSupport::Cache::NullStore
  class InvalidDataException < Exception; end

  private

  def write_entry(key, entry, **options)
    check_value(entry.value)
    true
  end

  def check_value(value)
    if value.is_a?(ActiveRecord::Base) || value.is_a?(ActiveRecord::Relation)
      raise InvalidDataException, 
        "Tried to store #{value.class} in a cache. We cannot do this because \
        the behaviour/schema may change between this value being stored, and \
        it being retrieved. Please store IDs instead."
    elsif value.is_a?(Array) || value.is_a?(Set)
      value.each { |v| check_value(v) }
    elsif value.is_a?(Hash)
      value.values.flatten.each { |v| check_value(v) }
    end
  end
end

With this in place, when your build runs any test that exercises behaviour that ends up trying to persist ActiveRecord objects into the cache will raise an exception, causing the test to fail with an explanatory message.

You can extend this to include other classes if you have other objects that may change their interface over time.

Double bonus: tests for NullStoreVerifyingAcceptableData

How can we be confident that the new cache store is actually going to warn us about behaviours in the test? What if it never ever raises the exception?

When I’m introducing non-trivial behaviour into my test suite, I like to test that too. So here’s some tests to sit alongside this new cache store, so we can be confident that we can actually rely on it.

require 'test_helper'

class NullStoreVerifyingAcceptableDataTest < ActiveSupport::TestCase
  setup do
    @typical_app_class = Widget
  end

  test 'raises exception when we try to cache an ActiveRecord object' do
    assert_raises_when_caching { @typical_app_class.first }
  end

  test 'raises an exception when we try to cache a relation' do
    assert_raises_when_caching { @typical_app_class.first(5) }
  end

  test 'raises an exception when we try to cache an array of ActiveRecord objects' do
    assert_raises_when_caching { [@typical_app_class.first] }
  end

  test 'raises an exception when we try to cache a Set of ActiveRecord objects' do
    assert_raises_when_caching { Set.new([@typical_app_class.first]) }
  end

  test 'raises an exception when we try to cache a Hash that contains ActiveRecord objects' do
    assert_raises_when_caching { {value: @typical_app_class.first} }
  end

  test 'does not raise anything when caching an ID' do
    Rails.cache.fetch(random_key) { @typical_app_class.first.id }
  end

  test 'does not raise anything when caching an array of IDs' do
    Rails.cache.fetch(random_key) { @typical_app_class.first(5).pluck(:id) }
  end

  private

  def assert_raises_when_caching(&block)
    assert_raises NullStoreVerifyingAcceptableData::InvalidDataException do
      Rails.cache.fetch(random_key, &block)
    end
  end

  def random_key
    SecureRandom.hex(8)
  end
end

If you extend the cache validator to check for other types of objects, you can add tests to make sure those changes work as you expect.

Triple-bonus code: what if you already have cached objects?

My real fix for WidgetCorp would actually try to mitigate this issue while still respecting the (possibly broken) objects still in the cache:

module WidgetHelper
  def available_widgets
    ids_or_objects = Rails.cache.fetch('available-widets', expires_at: Date.tomorrow.beginning_of_day) do
      Widget.available.pluck(:id)
    end
    ids = if ids_or_objects.first.present? && ids_or_objects.first.is_a?(ActiveRecord::Base) 
      ids_or_objects.map(&:id)
    else
      ids_or_objects
    end
    Widget.where(id: ids)
  end 
end

This allows us to still use the ActiveRecord instances that are cached, while storing any new data in the cache as just IDs. Once this code has been running for a day, all of the existing data in the cache will have expired and the implementation can be changed to the simpler one.

  1. Pocket dimension, closed timelike curves, above your paygrade, don’t worry about it! 

  2. You might imagine we could get around all this by caching the view, and indeed in this case, it wouldn’t raise the same problems, but in a typically mature Rails application we end up using cached data in other places outside of view generation, so the overall point is worth making. Still, award yourself 1 gold credit. 

Two thoughts about maintainable software

I really enjoyed the most recent episode of Robby Russell’s podcast “Maintainable”, in which Glenn Vanderburg1 talks about capturing explanations about code and systems, along with managing technical debt. It’s a great episode, and I highly recommend pouring it into your ears using whatever software you use to gather pods.

The whole thing is good, but there were two specific points brought up fairly early in their conversation that got me thinking. What a great excuse to write a post here, and make me feel less guilty about potentially never publishing the Squirrel Simulator 2000 post I had promised.

I know; I’m sorry. Anyway, here’s some tangible words instead of that tease. Enjoy!

Documenting the why

At the start of the conversation, Robby asks Glenn what he thinks makes “maintainable” software, and part of Glenn’s response is that there should be comments or documents that explain why the system is designed the way it is.

Sometimes things will be surprisingly complex to a newcomer, and it’s because you tried something simple and you learned that that wasn’t enough. Or sometimes, things will be surprisingly simple, and it’s good to document that “oh, well we thought we needed it to be more complicated, but it works fine for [various] reasons.”

Robby goes on to ask how developers might get better at communicating the “why” – should it be by pointing developers at tickets, or user stories somewhere, or it should be using comments in the code, or maybe some other area where things get documented? And without wanting to directly criticise Glenn’s answer, it brought to mind something that I have come to strongly believe during my career writing software so far: the right place to explain the why is the commit message.

But why not comments?

I’ve lost count of the number of times that I’ve found a comment in code and realised it was wrong. We just aren’t great at remembering to keep them updated, and sometimes it’s not clear when a change in the implementation will reduce the accuracy of the comment.

But it’s already fully explained in the user story/Basecamp thread/Trello card…

I’ve also lost count of the number of times that a commit message contains little more than a link, be it to a defunct Basecamp project, or a since-deleted Trello card, or am irretrievable Lighthouse ticket, or whatever system happened to be in use ten years ago, but is almost certainly archived or gone now, even if the service is still running.

And even if we are lucky and that external service is still running, often these artefacts are discussions where a feature or change or bug is explored and understood and evolved. The initial description of a feature or a bug might not reflect the version we are building right now, so do we really want to ask other developers and our future selves to have to go back to that thread/card/ticket and re-synthesise all those comments and questions, every time they have a question about this code?

Put it in the commit message

Comments rot. External systems, over the timescales of a significant software product, are essentially ephemeral. For software to be truly maintainable, it needs to carry as much of the explanation of the why along with it, and where the code itself cannot communicate that, the only place were an explanation doesn’t rot is where it’s completely bound to the implementation at that momment in time. And that’s exactly what the commit message is.

So whenever I’m writing a commit, I try and capture as much of the “why” as I can. Yes, I write multi-paragraph commit messages. The first commit for a new feature typically includes an explanation of why we’re building it at all, what we hope it will do and so on. Where locally I’ve tried out a few approaches before settling on the one I prefer, I try and capture those other options (and why they weren’t selected) inside the commit message for the approach I did pick.

Conversely, if I am looking at a line of code or a method and I’m not sure about the why of it, I reach for git blame, which immediately shows me the last time it was touched, and (using magit or any good IDE) I can easily step forward and back and understand the origin and evolution of that code – as long as we’ve taken the time to capture the why for each of those changes. And this applies just as much to the code that I wrote as it does to other developers. Future-me often has no idea why I made certain choices – sometimes I can barely remember writing the code at all!

So anyway, in a nutshell, when you’re committing code, try to imagine someone sitting beside you and asking “why?” a few times, and capture what your explanation would be in your commit message.

BONUS TIP: finding that your “why” explanation is a bit too long? That’s probably a signal that you need to make smaller commits, and take smaller steps implementing your feature. I’ve never regretted taking smaller steps, even though I recognise sometimes the desire to “just get it done” can be strong.

Using tests as TODOs

A little later, the conversation turns to the merits of “TODO” comments in the code, and how to handle time-based issues where code needs to exist but perhaps only for a certain amount of time in its current form, before it need to be revisited. It’s not unusual for startups to need to get something into production quickly, even if they know that they are spending technical debt in doing so. Glenn correctly points out that it can be hard to make sure that comments like that get attention in the future when they should.

The problem is that when you put them in there, there’s no good way to make sure you pay attention when the time comes [to address the debt]

I spend a fair bit of my professional time working with a startup that has a very mature (13 years old) Rails codebase, and more often than not, moving quickly involves disabling certain features temporarily, as well as adding new ones. For example, on occasion we might need to disable the regular newsletter mechanism and background workers while the company runs a one-off marketing campaign. To achieve this, we need to disable or modify some parts of the system temporarily, but ensure that we put them back in place once the one-off campaign is completed.

In situations like this, I have found that we can use the tests themselves to remind us to re-enable the regular code. Here’s a very simple test helper method:

def skip_until(date_as_string, justification)
  skip(justification) unless Date.parse(date_as_string).past?
end

Becase we have tests that check the behaviour of our newsletter system under normal conditions, when we disable the implementation temporarily, these tests will naturally fail, but we can use this helper to avoid the false-negative test failures in our continuous-integration builds, without having to delete the tests entirely and then remember to re-add them later:

class NewsletterWorkerTest < ActiveSupport::TestCase
  should "deliver newsletter to users" do
    skip_until('2020-12-01', 'newsletters are disabled while Campaign X runs')
    
    # original test body follows
  end
  
  # other tests 
end

This way, we get a clear signal from the build when we need to re-enable something, without the use of any easy-to-ignore comments at all.

Not all comments?

I’m not completely opposed to all comments, but it’s become very clear to me that they aren’t the best way to explain either what some code does, or why it exists. If we can use code itself to describe why something exists, or is disabled – and then remind us about it, if appropriate – then why not take advantage of that?

And if there’s only one thing you take away from all the above, let it be this: take the time to write good commit messages, as if you are trying to answer the questions you could imagine another developer asking if they were sat beside you. You’ll save time in code reviews, and you’ll save time in the future, since you’ll be able to understand the context of that code, the choices and tradeoffs that were made while writing it – whe why of it – much faster than otherwise.

Here’s a great talk that further illustrates the value of good commit messages, and good commit hygiene in general: A Branch in Time by Tekin Süleyman.

  1. I met Glenn once, at a dinner during the Ruby Fools conference (such a long time ago now). He said some very nice things to me over that dinner, which I’ve never forgotten. So thanks for that, Glenn! 

LGTM

There are a number of techniques that remote teams can use to collaborate, but the main one the client1 relies on is code review via pull requests.

Code review on pull requests can help catch potential issues, but it’s not as good as actual TDD (or any other conscientous testing practice) because once implementation is written, it’s hard to evaluate what aspects of it are intended behaviour and which are side effects, or even bugs.

Code review on pull requests can be good for getting a second opinion on a specific implementation, but they’re not as good as pairing, where ideas and approaches can be discussed and evaluated before any investment is made, and hard-to-spot edge cases can be identified before they are committed into code.

Code review on pull requests is good for communicating between developers about what everyone is working on, but they’re not as good as actual discussion, ideally documented, because without conscious effort to capture context – the why this is being built, and why in this particular way – in either the tests, the commit messages, the pull request description, or ideally all three, it’s often up to the reviewer to try and reverse-engineer the intent and original goals of the piece of work. It’s easy to underestimate how much effort that reverse-engineering can be.

Code review on pull requests is good for avoiding unending, ambigious conversations about what might work to solve a problem, because it’s always easier to measure the value of something concrete than something hypothetical, but by the time the review happens there’s already investment in that particular solution and that particular implementation of that solution. I see the same thing with MVPs, which should be built quickly without particular care, then learned from and finally discarded, but instead are built quickly and without particular care, but then put into production to become the foundation for future work… but I’m digressing; the point is that we developers tend to believe that extant code has more value than it really does, and the momentum of that particular approach can override any fundamental questions2 that a code review might raise.

But… getting good at testing requires effort, and pairing requires a lot more energy than hacking solo, and thoughtfully documenting takes time, and development zen-like detachment from the fruits of our labour doesn’t come naturally either. And all these things require practice, and who has time to practice when there are features to ship!

So we do code reviews on pull requests. LGTM3 :(

  1. At the moment, I spend most of my active development time working remotely for a single client. It’s a Rails project that’s been running for approximately 13 years, with around 5 developers of varying skill levels actively contributing to the backend, where I spend most of my time. 

  2. “Did you consider, maybe, not implementing this at all?” 

  3. “Looks Good To Me”, the hallmark sign that a reviewer has not even checked out and run the code, let along read it carefully and thought about more than syntax. 

I am an omakase chef

I run a restaurant with a single seat, and a single sitting, normally around 10pm.

There is only one item on my menu.

At that time, my sole patron arrives and sits, watching me while I set to work, my hands moving slowly and deliberately with the practice of hundreds of prior repetitions.

The pill is carefully broken in half, and the remainder stashed back in it’s blister.

A small piece of mouldable savoury putty is selected, and carefully shaped around the pill, such that it fully surrounds it, but with the pill off-center, leaving a larger amount of putty at one end.

A Dreamie1 is selected, and is carefully split, and one half is held back in the hand while the other is delicately moulded into the waiting putty, such that the whole can now sit with the Dreamies-half on top, sitting on the putty with the pill hidden directly beneath it. This completes the dish. It is a Dreamie nigiri.

The small morsel is finally placed in front of the diner, who considers it briefly, then eats, and then leaves.

  1. The singular of “Dreamies” is basically unknowable. 

Back on the wagon

Hey, it’s been a while. Sorry about that.

For the past few months I’ve increasingly felt the tug to express thoughts and such, but while most of the world now does that on Twitter, as with Facebook I don’t feel like I can participate on there without condoning some of the awful aspects of social media.

So I’m resurrecting this site, in the hopes that I can write a bit more here to scratch that itch.

Obviously there was no way I could possibly resume writing here without a total overhaul of the design and backend. And then having spent days on it, learning Tachyons in the process, I flicked back to the previous design and, y’know what, it actually wasn’t that bad. But anyway, we’re here now.

Speaking of Tachyons, I did read an interesting article about CSS utility classes and “separation of concerns” that has finally helped me understand the motivations that have driven front-end developers away from “semantic” CSS. As primarily a backend developer, I struggle to shake the dream of writing simple markup for a “thing” and then having different CSS render it appropriately in whatever contexts it might appear, but this helped me understand the journey. I want to re-read that article, because I have a hunch that the destination it describes isn’t actually that far away from the “semantic” one I hope for, just that the CSS might end up being dynamically composed from utility classes by a pre-processor. Maybe.

Some random thoughts I’ve had on return to this site and the codebase behind it:

Anyway, I’m back. Or am I? I think I am. I hope I am.

I’m back.