Making number checks pretty with unbounded ranges and formatting

This code showed up in a discussion yesterday:

switch averagePosts {
        case 10000000...25000000: return .d
        case 5000000...10000000: return .c
        case 2500000...5000000: return .b
        case 500000...2500000: return .a
        case 250000...500000: return .b
        case 100000...250000: return .c
        case 50000...100000: return .d
        default: return .f
}

I thought it would be much cleaner and easier to use if the coder incorporated unbounded ranges and used Swift’s built-in underscore formatting for their numbers:

switch averagePosts {
case 10_000_000...: return .d
case  5_000_000...: return .c
case  2_500_000...: return .b
case    500_000...: return .a
case    250_000...: return .b
case    100_000...: return .c
case     50_000...: return .d
default:            return .f
}

A few things to note about this rewrite:

Inserting the underscores and aligning the numbers greatly simplifies code validation. Normally I’m not a proponent of making code look “pretty” by adding spaces. Here it serves a very specific purpose, allowing eyeball checks of each number with respect to its fellows.

I kept colon-hugging with default but spaced out the return value to provide a uniform return column for the same reason.

My logic is slightly different from the original as I don’t check for numbers exceeding 25 million. If you wanted to add that, the case would have to be 25_000_001 and not 25_000_000 to exclude the valid value that returns .d. returning .f (the default) instead.

This is a good example of where a full set of Swift range operators would help. Using 25_000_000<..., which would be basically the unbounded half-open-at-the-start operator, could exclude 25 million without using the uglier “I have to add one” hack, which is an example of “have to stop and think” code, which is best avoided.

A full set of operators like those I mentioned in this post would have to be updated for Swift’s new support of unbounded ranges:

  • <... and ...< half open unbounded
  • ... closed (2 values) or unbounded (1 value) or all numbers of type (0 values)
  • <.. and ..< half open, bounded
  • <.< fully open, bounded

One Comment

  • As a general rule, I’m against using whitespace to align things in this way.

    Firstly, it makes changes unnecessarily noisy. Consider what happens when you add a 100m case – instead of a one line diff to add a single case, you’ve got an eight line diff that adds a single case and changes seven other cases.

    This then has a knock-on effect. Consider what happens when another developer has made a change to one of those cases. You have now caused a merge conflict where there didn’t need to be one.

    It also breaks blame. If I want to know why one of the other cases was introduced/modified, blame will now tell me that it was the commit that added the 100m case rather than the commit that actually introduced/modified it.

    I agree, it looks neater. All other things being equal, I’d go with that too. But less readable diffs, unnecessary merge conflicts, and a broken blame are not a good trade-off for that.