Update 2024-05-20: The originally published article contained a factual error in the example about reading from a channel, which has been corrected. The author regrets the error.

Introduction

There's an old joke among programmers that there are only 2 hard problems in computer science: naming things, concurrency, and off-by-one errors.

Let's talk about naming boolean variables in Go.

ok

Ok yeah let's talk. But actually I want to talk about the following idiom.

	m := make(map[string]struct{})
	m["a"] = struct{}{}
	
	if _, ok := m["a"]; ok {
		fmt.Print("a exists in the map")
	}

Just like there's an unwritten law that every error variable in Go must be named err, there's an unwritten law that every map existence variable in Go must be named ok. If you don't do this you'll never be invited to Commander Pike's birthday parties, you'll start attracting snide comments on your pull requests, and the cool kids will treat your part of the code base like a sick raccoon curled up in there and died a few months ago. Naming map existence bools something other than ok just isn't done.

ok: Not for just maps

The ok idiom has extended out of strict map existence checks, into other related areas. For example, Dolt's code base has many methods with a signature like this:

func GetTable(ctx context.Context, tName TableName) (*Table, bool, error)

The boolean return values is true if a table with the given name exists in the database, false otherwise. Now, separating out an existence check from an error condition is a good thing, actually. You never want to force clients to check for a particular error type in business logic, like this:

func GetTable(ctx context.Context, tName TableName) (*Table, error)

...

table, err := GetTable(ctx, name)
if errors.is(err, ErrNotFound) {
	// this is not a real error
	return nil
} else if err != nil {
	return err
}

That's generally an anti-pattern. An error should by default be considered non-recoverable, to be returned when something goes very wrong. Semantically, a table not existing isn't really an error, it's something you expect to happen all the time. An interface that returns an error in the course of normal operation forces clients to understand a lot of details to use the interface correctly (looking at you, io.EOF). And errors can be expensive to construct and examine, so you don't want them constructed or interpreted on your hot path.

On the other hand, it's also something of an anti-pattern to return nil for both the primary return value and the error return value. Clients expect that if err is nil, then the other result won't be.

So we compromise: we return a boolean existence result alongside the primary result, which makes it obvious to a client that the primary return value can be nil in the course of normal operation. This lets us write the following very idiomatic code:

func GetTable(ctx context.Context, tName TableName) (*Table, bool, error)

...

table, ok, err := GetTable(ctx, name)
if err != nil {
	// A error definitely occurred!
	return err
}

if !ok {
	// We decide if non-existence is an error or not at the call site!
	return handleNotFound(name)
}

// Definitely not a nil pointer!
handleTableLookup(table)

So what's the problem? Well, the problem is in our naming. ok is a bad name.

ok Considered Harmful

You shouldn't name your existence variables ok, even though everybody does it. Yes, I mean it.

Consider the following code, which constructs a three-way-merger for tables in Dolt.

func (rm *RootMerger) makeTableMerger(ctx context.Context, tblName string) (*TableMerger, error) {
	tm := newTableMerger(rm)

	var ok bool
	var err error

	tm.leftTbl, ok, err = rm.left.GetTable(ctx, tblName)
	if err != nil {
		return nil, err
	}
	if ok {
		if tm.leftSch, err = tm.leftTbl.GetSchema(ctx); err != nil {
			return nil, err
		}
	}

	tm.rightTbl, ok, err = rm.right.GetTable(ctx, tblName)
	if err != nil {
		return nil, err
	}
	if ok {
		if tm.rightSch, err = tm.rightTbl.GetSchema(ctx); err != nil {
			return nil, err
		}
	}

	tm.ancTbl, ok, err = rm.anc.GetTable(ctx, tblName)
	if err != nil {
		return nil, err
	}
	if ok {
		if tm.ancSch, err = tm.ancTbl.GetSchema(ctx); err != nil {
			return nil, err
		}
	}

	return &tm, nil
}

Here we're using the standard idiom of naming our existence variable ok. We can't use variable shadowing with := because we're assigning into a struct field, so err and ok get assigned three different times.

Is this so bad? Well, I suppose it could always be worse. But consider what happens when someone naively attempts a refactoring to shuffle parts of the business logic around, maybe to extract some methods.

func (rm *RootMerger) makeTableMerger(ctx context.Context, tblName string) (*TableMerger, error) {
	tm := newTableMerger(rm)

	var ok bool
	var err error

	// get left, right, and anc tables
	tm.leftTbl, ok, err = rm.left.GetTable(ctx, tblName)
	if err != nil {
		return nil, err
	}

	tm.rightTbl, ok, err = rm.right.GetTable(ctx, tblName)
	if err != nil {
		return nil, err
	}

	tm.ancTbl, ok, err = rm.anc.GetTable(ctx, tblName)
	if err != nil {
		return nil, err
	}

	// Assign the schemas 
	if ok {
		if tm.leftSch, err = tm.leftTbl.GetSchema(ctx); err != nil {
			return nil, err
		}
	}
	if ok {
		if tm.rightSch, err = tm.rightTbl.GetSchema(ctx); err != nil {
			return nil, err
		}
	}
	if ok {
		if tm.ancSch, err = tm.ancTbl.GetSchema(ctx); err != nil {
			return nil, err
		}
	}

	return &tm, nil
}

This code still compiles but it's now nonsense. Hope your code reviewer is feeling alert today, and that you wrote tests for the conditions in each of those three independent blocks.

As Go programmers we are used to err != nil blocks being married to the line of code above, but ok checks somehow seem more "separable." They're not. And that's a bad thing.

As with reusing err across assignments, variable shadowing makes this all worse, since the same name can refer to two different variables in different parts of the same function. It also makes simple refactorings unstable: change a line of code here, silently break something seemingly unrelated 20 lines away.

"But that's a fake example" you might object. a) It's not, I perform this kind of cleanup across our code base continually, and b) it's trivial to find examples like this. I don't want to name names, but I've found code that looks like this, checked into our codebase, running in production:

	m := make(map[string]TableName)
	
	tableA, ok1 := m["A"]
	tableB, ok2 := m["B"]
	
	if ok1 && ok2 {
		fmt.Println("Both A and B are present")
	}

Here we have two existence check variables that need to be evaluated at the same time, so they need different names, and we've chosen terrible ones. "But that's just bad naming" you object, pushing your glasses up your nose. Yes, but it's terrible naming that falls directly out of the ok convention. The function started with a single ok variable and all was well, and then someone made a change that added a second, and they had so strongly internalized the ok convention that this seemed like a good idea to them.

And then yet another person reviewed the change, and they had also internalized the ok convention so strongly that they shrugged and said "ship it."

We don't have to live like this!

A better alternative exists

This polemic came to mind because I recently reviewed a PR where my talented and handsome colleague Jason made some changes to the three-way marge code above, and in the process fixed the naming. Tell me this isn't better in every way.

func (rm *RootMerger) makeTableMerger(ctx context.Context, tblName string, mergeOpts MergeOpts) (*TableMerger, error) {
	tm := newTableMerger()
	
	var err error
	var leftSideTableExists, rightSideTableExists, ancTableExists bool

	tm.leftTbl, leftSideTableExists, err = rm.left.GetTable(ctx, doltdb.TableName{Name: tblName})
	if err != nil {
		return nil, err
	}
	if leftSideTableExists {
		if tm.leftSch, err = tm.leftTbl.GetSchema(ctx); err != nil {
			return nil, err
		}
	}

	tm.rightTbl, rightSideTableExists, err = rm.right.GetTable(ctx, doltdb.TableName{Name: tblName})
	if err != nil {
		return nil, err
	}
	if rightSideTableExists {
		if tm.rightSch, err = tm.rightTbl.GetSchema(ctx); err != nil {
			return nil, err
		}
	}
	
	// Reverify constraints if asked to
	if mergeOpts.ReverifyAllConstraints {
		if !leftSideTableExists && rightSideTableExists {
			// if left side doesn't have the table... stub it out with an empty table from the right side...
			tm.leftSch = tm.rightSch
			tm.leftTbl, err = doltdb.NewEmptyTable(ctx, rm.vrw, rm.ns, tm.leftSch)
			if err != nil {
				return nil, err
			}
		} else if !rightSideTableExists && leftSideTableExists {
			// if left side doesn't have the table... stub it out with an empty table from the right side...
			tm.rightSch = tm.leftSch
			tm.rightTbl, err = doltdb.NewEmptyTable(ctx, rm.vrw, rm.ns, tm.rightSch)
			if err != nil {
				return nil, err
			}
		}
	}

	tm.ancTbl, ancTableExists, err = rm.anc.GetTable(ctx, doltdb.TableName{Name: tblName})
	if err != nil {
		return nil, err
	}
	if ancTableExists {
		if tm.ancSch, err = tm.ancTbl.GetSchema(ctx); err != nil {
			return nil, err
		}
	} else if schema.SchemasAreEqual(tm.leftSch, tm.rightSch) && tm.leftTbl != nil {
		// If left & right added the same table, fill tm.anc with an empty table
		tm.ancSch = tm.leftSch
		tm.ancTbl, err = doltdb.NewEmptyTable(ctx, rm.vrw, rm.ns, tm.ancSch)
		if err != nil {
			return nil, err
		}
	}

	return &tm, nil
}

Look at that, variable names that clearly express their contents and the author's intent, what an idea! Besides being easier to read and understand, this code is now also much easier to refactor as we add features: you can move the new ReverifyAllConstraints block to another part of the function to execute later, and everything still works.

The new code uses a much better convention for existence check bools than the ok convention: the exists convention. Here's a minimal example:

	m := make(map[string]TableName)
	
	...
	
	if table, tableExists := m["a"]; tableExists {
		fmt.Printf("Table %v exists\n", table)
	}

It's so simple and so good. You spent an appropriate amount of care naming your primary variable (you did take an appropriate amount of care naming your primary variable, right Anon?). For its accompanying existence check variable, you don't need to think: just append Exists onto the end. Now your code reads more clearly, is more robust in the face of future changes, and all it cost you was a few keystrokes.

Variation: optional channel receives

Another place you commonly see the use of ok as a naming convention in Go programs is when receiving from a channel which might be closed. The following code prints 0 if the channel was closed, even though the value 0 was not sent on the channel:

	c := make(chan int)

	...

	val := <-c
	fmt.Printf("val: %d", val)

For those times when you are reading from a channel that might be closed and want to distinguish getting a real value from the zero value for that type, you can receive from the channel with a boolean, which will immediately return with a false value if the channel is closed.

	c := make(chan int)

	...
	
	val, ok := <-c
	if ok {
		fmt.Printf("val: %d, received a value: %v", val, ok)
	}

Here as well, using the name ok can cause problems down the line, for all the same reasons as when using it for existence checks. And if you're also using ok for existence checks in the same functions, these problems just compound.

The exists pattern doesn't quite fit here. Instead, why not use a name like received?

	val, valReceived := <-c
	if valReceived {
		fmt.Printf("val: %d", val,)
	}

Clear and unambiguous, and won't break on accident when statements are reordered.

But I like using ok

There's a counterargument here: conventions don't derive their power from being correct, they derive their power from being commonly used and understood. We now know that electrical current flows from the negative terminal in a battery to the positive one, but the convention is the opposite and we aren't about to change it. So why should we change the convention on existence check variables? Most of the time there's only one ok per block, so it doesn't matter. Won't it just confuse people?

Here I think it should be obvious that the benefits of my proposed replacement convention far outweigh the costs.

1. exists and received are perfectly clear whether you have been exposed to the ok convention or not
2. Starting with a single xExists name means there's no chance you'll be reusing it for yExists later in the function, avoiding the pitfalls of variable shadowing altogether

All that said: a foolish consistency is the hobgoblin of small minds, and I'm no tyrant.

Like most programming style guides, moderation is key. If you have a single use of an ok existence variable in a small function, it's probably fine. Nobody is coming for your variable.

func TableSize(tables map[tableName]Table, name tableName) int {
	table, ok := tables[name]
	if !ok {
		return 0
	}
	return table.Size
}

But if in the future you need to write a second existence check in the same function? Do the right thing, and rename them with the exists convention.

An exception: type assertions

The main issue that makes the ok convention problematic is the tendency for such variables to escape their narrow contexts.

But there's another pattern that uses ok where this problem is much less likely to arise: checked type assertions.

subInterface, ok := x.(SubInterface)
if ok {
	subInterface.DoMore()
}

Unlike existence checks, the use of ok here is a fine convention. The name itself makes more sense in this context (the assertion succeeded, it literally went ok). It's hardly ever the case that the ok variable needs to divorce itself from the line of code immediately above where it's declared. And if you use the exists convention for your existence checks, then there's no danger that ok will be misinterpreted in this much narrower use even if you move code blocks around.

So keep using ok in type assertions, I won't think less of your code. Some authors use the convention isSubInterface for the boolean value here, which is also a great choice in longer functions where ok might be ambiguous later in the function.

Doesn't this apply to err as well?

Y'all aren't ready for that conversation.

Conclusion

We're building Dolt, the world's first version-controlled SQL database. We have been writing it for over five years now, and our codebase is just as riddled with ok checks as yours probably is. But we are older now, and wiser. And better looking, thanks for noticing.

The ok convention is bad, and we can do better. I've been encouraging the rest of the engineering team to use the exists convention in PR reviews for a while now, and will redouble my efforts after making them all read this essay.

Have questions or comments about Go naming conventions? Or maybe you are curious about the world's first version controlled SQL database? Join us on Discord to talk to our engineering team and other Dolt users.