pragmatist

Patrick Joyce's Website

The Genius of Uber

Photo from Rob Nguyen on Flickr

The on-demand car-service company Uber has inspired much deserved admiration. Many new companies are trying to be the “Uber for X.” Most of them will fail, because most new companies fail. But I think many of them don’t even stand a chance because they don’t understand what makes Uber special.

Previously-Covered Misunderstandings

Jeff Morris, Jr. explored some of the reasons these companies are doomed a few weeks ago in a post on Medium.

His first point was that people starting “Uber for X” see the success Uber has had with a mobile-first strategy and think they must focus on an app. App-first works for Uber because they’re an inherently mobile offering; if you need a ride your often not sitting at your desk or on your couch—and even if you are—you still need to tell the driver where to pick you up.

What’s so special about car-services?

Uber has great execution. The app is simple to understand and the service is a pleasure to use.

However, Uber has been successful not only because of their great execution. They also picked a great market. The car service business was ideally suited for a company like Uber to upend.

Let’s look at the nature of the car service business pre-Uber:

1. Many, Independent Service Providers

Most car services are small firms with just a few cars. Many are single-car owner/driver operations. This means that Uber can’t be blocked by a single powerful gatekeeper.

Its the difference between selling consumer software versus selling enterprise software. Because there are many, small service provider no one car service company has the power to dictate terms to Uber.

2. Anonymous, Commoditized Transactions

With many other service providers there is a bit of a learning curve. With a cleaning service you’ll need to explain where everything goes and what you want done. After that initial setup, you’ll want to minimize future transaction costs by having (ideally) the same crew or (at least) the same company come back again.

In contrast, when I call for a car (or taxi) I don’t really care who the driver is. As long as they get me where I need to go quickly and comfortably I’ll be happy. Any car that meets Uber’s standards will do.

This anonymity is desirable to the drivers as well. At any given moment, the driver wants to find a fare that is close to where they are; not someone they happen to have picked up before.

This keeps incentives aligned between Uber and the drivers. Drivers won’t try to “steal” Uber customers by asking them to call directly in the future. On the other hand a cleaning service that is referred by an “Uber for Cleaning Services” will have an incentive to try to convince me to book directly with them in the future.

Companies like Cherry and Exec went so far as to hire staff to combat this, but growing and managing a workforce is a much harder (and less lucrative) problem than connecting buyers and sellers.

3. Idle Resources with Low Marginal Cost

Previously, car services were only booked in advance and by people willing to pay a significant premium. This means many car services had very low utilization rates. They would be booked for a few hours most nights, but some nights wouldn’t have any business.

Uber helps drivers find business for those times that they otherwise would be idle. Drivers love this as they can make more money without throwing away their established business.

4. Regulatory Inefficiencies

Car services (and taxis—which are Uber’s real competition) are a regulated industry. The number of taxis in New York city is determined—not by the free market—but by a commission. Whether taxis in DC will accept credit cards is decided by the government.

In pretty much every market Uber enters they are violating those regulations.

And Uber. Does. Not. Give. A. Fuck.

Uber is betting that before the regulators can shut them down they’ll be able to build enough goodwill with their customers to generate public pressure to keep them open. And so far they’ve been right.

What we should a learn from Uber

Be a marketplace, not a service provider. Hiring and training people to wash cars or clean houses doesn’t scale particularly well.

Choose your market carefully. Pick a market where your incentives align with your service providers.

Choose a product strategy that fits your market. If your chosen market is on-demand and location-centric then building a app first makes sense. On the other hand if your market requires research and scheduling appointments a web-first approach might be a better idea.

Be willing to make some enemies. Powerful entrenched players or government regulation can be a significant deterrent to enter a market. The good news is that if you’re willing to piss off the existing player or the government you’ll be rewarded with a valuable market with less competition than you would naturally expect.

Hey, ignoring regulations for as long as possible worked for PayPal.

The Slow Android Upgrade Curve Is a Real Problem

Tim Bray wrote a post on Tuesday agruing that Google Play Services means that the slow upgrade curve for Android “matters less and less for developers”. He concludes:

Yeah, if what you care about is new smoother glass and slicker chips and faster broadband, you’re still on the dessert schedule. But if what matters is what apps can do, you can pretty well ignore that Versions dashboard.

I can’t imagine how frustrating it must be to have worked on Android and see 45% of people stuck using a version that is more than 2 years old (particularly when more than 80% of iOS users are on the 5 month old iOS 6.) But to argue with a straight face that dealing with out of date Android versions isn’t a real problem for people who care about “what apps can do” is complete and utter bullshit.

The differences between 2.3 and 4.0 with regards to HTML and CSS support are very real. The performance differences for HTML rendering are very real (as Google brags about when talking about ICS)

I recently overheard a developer say “Android 2.3 is the new IE6” after fighting with a bug that only manifested in 2.3. Let’s put it this way: If you’re being compared to IE6 something has gone horribly wrong. Google Play Services seem to be a clever way to get around carriers and handset manufacturers refusing to issue updates. But if people in the Android world can’t see that it is a major problem for the most common version of Android to be over two years and two major releases out of date then the problem will never get solved.

Calculating Sample Sizes for AB Tests With Vanity (and R)

You want to run an AB test. How many participants do you need in your test?

As always, the answer is “it depends”. In this case, it depends on:

  1. What your base conversion rate is.
  2. How large of a difference you want to be able to detect.
  3. How concerned you are about Type I and Type II errors (false positives and false negatives)

There is no generic rule of thumb. Don’t trust any advice like “you want about 3,000 people in the test to be confident.” The correct sample size always depends on these 3 parameters for your specific test.

Basically:

  • The lower the base conversion rate the more participants you’re going to need
  • To detect smaller differences you’re going to need more participants
  • If you want to increase your confidence in your result, you guessed it, you’re going to need more participants.

How to calculate necessary sample size

If you know your base conversion rate and what size difference you wish to detect it is easy to calculate the necessary sample size using R.

> power.prop.test(p1=0.25, p2=0.275, power=0.8, alternative='two.sided', sig.level=0.05)

     Two-sample comparison of proportions power calculation 

              n = 4861.202
             p1 = 0.25
             p2 = 0.275
      sig.level = 0.05
          power = 0.8
    alternative = two.sided

 NOTE: n is number in *each* group

So, in an ideal world you would run all tests as follows:

  1. Track your base conversion rate For example, 25% of people who reach a registration page successfully register.
  2. Agree on the size of the difference we want to detect We may only care about detecting relative differences of 10% or more (27.5% or better conversion using the example above)
  3. Decide on the desired significance level. This is the chance of a false positive. It is common to use 0.05 (which represents a 5% chance of a false positive)
  4. Decide on the desired statistical power. This is the chance of a false negative. It is common to use 0.80 (which means that if there is a difference there is a 20% chance we’ll miss it)
  5. Calculate the necessary sample size as described above. Using these examples we would need to have 4862 people in each group.
  6. Run the test until you have enough participants in both your control and treatment Don’t look at the results while the test is running
  7. End the test
  8. Analyze the test

Unfortunately, that isn’t how it normally goes in the real world:

  • We often don’t know what the baseline conversion is. Often times conversion rates for the control aren’t clearly tracked until you start the test. Sometimes, you’re unable to effecively baseline a conversion rate because it varies wildly. I have a little bit of experience dealing with optimizing ecommerce sites where inventory is only available for a limited time. The quality of inventory can have a large effect on the conversion rate, so it is very difficult to compare conversion rates across time.
  • Most AB Testing software provides real time results which make it easy to fall victim to repeated significance testing errors.

To combat these pitfalls we can use the control as an approximation of the true base conversion rate. Then we can use that as the base conversion rate to figure out how much longer we will need to run a test to detect a difference of the size demonstrated.

Further Reading

I am not a statistician. If you want to learn more go read Noah Lorang’s post about calculating sample sizes and Evan Miller’s explanation of repeated significance testing errors.

Link: Any Sufficiently Technical Expert Is Indistinguishable From a Witch

As Arthur C Clarke puts it, “Any sufficiently advanced technology is indistinguishable from magic”. Here is my corollary: “Any sufficiently technical expert is indistinguishable from a witch”. People fear magic they don’t understand, and distrust those who wield that magic. Things that seem reasonable to technical geeks seem illegal to the non-technical. The non-technical think they understand MAC addresses and address blocking, but they don’t. Thus, Aaron’s indictment might seem a fair interpretation of the law, but it’s a wholly unfair interpretation of technology.

The CFAA is hopelesly broad. Thanks to it I have broken federal law on countless occasions by doing things no sane person would consider a crime.

Reject Bang Considered Harmful

reject! has some (at least to me) unexpected behavior that can easily lead to bugs.

In the case where it makes changes to the array it returns the new, filtered value of the array, just like reject. However, in the case that it makes no changes it returns nil

reject
1
2
3
4
5
6
7
8
9
10
>> a = [1, 2, 3, 4]
=> [1, 2, 3, 4]
>> a = a.reject(&:odd?)
=> [2, 4]
>> a
=> [2, 4]
>> a = a.reject(&:odd?)
=> [2, 4]
>> a
=> [2, 4]
reject!
1
2
3
4
5
6
7
8
9
10
>> a = [1, 2, 3, 4]
=> [1, 2, 3, 4]
>> a.reject!(&:odd?)
=> [2, 4]
>> a
=> [2, 4]
>> a.reject!(&:odd?)
=> nil
>> a
=> [2, 4]

As long as you don’t store the return value or chain a method call on the reject! call you’re fine.

However, this is one of those rare cases where I think it is worthwhile to defensively code for potential future changes. I’ve seen several bugs introduced in production code where people come back later and start chaining method calls a la @collection.reject!(&:odd?).each{|d| ...

Therefore, even though it is more verbose I prefer to use the form @collection = @collection.reject(&:odd?)

Two Small OSX Settings Changes That Make Me Much Happier

RAM-only hibernate mode

~> pmset -g | grep hibernatemode
 hibernatemode        3
~> sudo pmset -a hibernatemode 0

This means that sleep and wake are instantaneous. The downside is that if I completely run out of power I’ll lose anything that isn’t saved. I have a bit of a tick about constantly saving things, so this doesn’t really worry me.

Goodbye Dashboard!

~> defaults write com.apple.dashboard mcx-disabled -boolean YES
~> killall Dock

Dashboard wasn’t really a problem for me on Snow Leopard but while using full screen mode in Mountain Lion I keep ending up on it accidentally. Also, the only thing I used it for was the calculator which I now do with Alfred.

Non-Even AB Testing Splits With Vanity

A question recently resurfaced on the vanity-talk mailing list about setting up tests with non-even splits.

Hey guys,

Wondering what people think about / how feasible it’d be to do the following:

Say I have a fairly experimental feature for my site that I’d like to
quickly implement and test. But, it’s really not ready for prime-time,
even 50% of my users would be too much. Maybe I want it to be closer
to 5% of my users, for various reasons (e.g. the software isn’t ready
for scale or I expect it to perform _worse_ but I want to ensure
that’s the case). I think it’d be great to have the definition of an
alternative be able to take a percentage of users who should see each
option and/or allow that to be changed via the dashboard.

As an added bonus, this could be a nice way to roll out new features
to your site quietly, testing for production/scale bugs more
gradually.

Thoughts?

This isn’t a feature of Vanity but it can be achieved by overriding the alternative_for method in your experiment definition like so:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
ab_test "your_ab_test" do
  description "Your AB test"

  metrics :conversions
  alternatives "control", "treatment"

  # Returns an index into the list of alternatives.
  #
  # The original alternative_for picks between alternatives evenly using
  #
  #   Digest::MD5.hexdigest("#{name}/#{identity}").to_i(17) % @alternatives.size
  #
  # This uses the same deterministic algorithm to put 95% in the control and 5% in
  # the treatement group. You can adjust the conditional to get different splits.
  def alternative_for(identity)
    Digest::MD5.hexdigest("#{name}/#{identity}").to_i(17) % 20 == 0 ? 1 : 0
  end
end

Caution

Running an AB Test with different sized treatment and control groups isn’t necessarily wrong but I would urge caution. If you’re dealing with relatively low conversion rates (for instance purchase rates in e-commerce funnels) you might find yourself tempted to change the ratio when the test is running to get the smaller group up to a significant size. Do not do this. This will most likely invalidate your test in one of two ways:

  1. If there is any seasonality in your conversion (for instance higher conversion on weekends vs. weekdays) you will change the relative populations of users and will no longer be able to compare treatment vs. control
  2. If the test has already been running for an extended period of time you may also invalidate your test because of the difference in the relative age of customers in the treatment vs. control groups. Vanity doesn’t have a time limit on conversion. That is, if your customer becomes part of the experiment today, and converts a week later they still count as a conversion. People who entered into the experiment in the past had have more time to convert, so by changing your split you may flood one of the groups with newer users and make it impossible to directly compare the two groups.

I would suggest giving each alternative an equal split but only for a subset of your users. You could accomplish this segmentation by segmenting your user population before testing like this:

vanity_helper.rb
1
2
3
4
5
6
7
def vanity_subgroup
  Digest::MD5.hexdigest("#{vanity_identity}").to_i(16) % 10
end

def in_your_ab_test?
  vanity_subgroup == 0 && ab_test(:your_ab_test) == "treatment"
end
experiment.rb
1
2
3
4
5
6
ab_test "your_ab_test" do
  description "Your AB test"

  metrics :conversions
  alternatives "control", "treatment"
end
Your Controller or View
1
2
3
4
5
if in_your_ab_test?
  # treatment
else
  # control
end

This also only puts 5% of the population in the treatment group. However, it also gives you roughly equal-sized control and treatment groups which I’ve found are much easier to understand and explain to non-technical stakeholders.

The vanity_subgroup method also allows you to run multiple simultaneous tests while keeping each test segmented to a different subset of your user population to prevent one test from affecting the results of another.

Rails 2.3 Fix for CVE-2012-2660: Unsafe Query Generation

Last Thursday the Rails security team announced CVE-2012-2660. This bug takes advantage of an issue in Rack that allows for a specially crafted request to set a param to [nil] instead of nil. This could circumvent checks for nil? and lead to unsafe query generation. See the bug report for more details.

Unfortunately for those of us still maintaining production apps on 2.3.X, the Rails team no longer supports the 2.X branches and did not release a patch.

Fortunately, it was pretty trivial to back port the patch from the 3.X branch to 2.3.14. Here is my commit: security/fix-unsafe-query-generation

Updating your application

If you’ve vendored rails

Just apply my patch to the rails source in your vendor directory.

If you are using bundler (HT Tom Ward)

  • Check out my branch. From within a clone of rails:
1
2
3
git remote add patrick https://github.com/KeeperPat/rails.git
git fetch patrick
git checkout patrick/security/fix-unsafe-query-generation
  • Build the gem files
1
2
3
4
5
6
7
8
9
10
11
12
13
cd actionmailer
rake gem PKG_BUILD=1
cd ../actionpack
rake gem PKG_BUILD=1
cd ../activerecord
rake gem PKG_BUILD=1
cd ../activeresource
rake gem PKG_BUILD=1
cd ../activesupport
rake gem PKG_BUILD=1
cd ../railties
rake gem PKG_BUILD=1
cd ..
  • Copy the *.gem files into your vendor/cache
1
cp **/pkg/*.gem <project-folder>/gems/cache
  • Update your gemfile to require rails 2.3.14.1
  • bundle update rails

If you are using bunder and have your own gem server (running geminabox)

The same as above except replace step 3 with:

  • Push the gem files to your geminabox
1
2
gem install geminabox
gem inabox **/pkg/*.gem

The Elements of Ruby Style: Predicate Methods

A method that returns true or false is called a “predicate method”.

In Ruby, there is a naming convention where predicate methods end with a question mark. For instance, if you have an Invoice class and are writing an instance method that returns whether the invoice has been paid or not, you would name the method paid?

This idiom helps improve readability by making Ruby code read more like English:

1
puts "This invoice has been paid" if invoice.paid?

Predicate methods should always return true or false. Because of the way that Ruby evaluates truthiness (false and nil are false; everything else evaluates to true) some people will advocate simply returning a “truthy” value. An implementation of Invoice#paid? that was truthy could look like this:

1
2
3
4
5
6
7
8
9
10
11
12
13
class TruthyInvoice
  attr_accessor :paid_at

  def paid?
    @paid_at
  end
end

invoice = TruthyInvoice.new
invoice.paid? # nil

invoice.paid_at = Time.now
invoice.paid? # 2012-03-24 20:45:59 -0400 

The above implementation of paid? will work fine in conditionals.

However, the intent of the paid? method is to tell you if the invoice has been paid, yet it is returning an instance of Time. This is confusing. The method would better represent the author’s intent if it always returned a boolean. The most common way to ensure that a method returns true or false is to use the not operator (!) twice. This is read “not not” and would be used as follows:

1
2
3
4
5
6
7
8
9
10
11
12
13
class Invoice
  attr_accessor :paid_at

  def paid?
    !!@paid_at
  end
end

invoice = Invoice.new
invoice.paid? # false 

invoice.paid_at = Time.now
invoice.paid? # true

Using “not not” only requires two more keystrokes and it more closely matches the intent of the method. One of the maxims of good programming is to “be liberal in what you accept and strict in what you emit”. Returning a boolean from predicate methods is being strict in what you emit.

The War on Comments Isn’t Over

Back in January, my very talented colleague Dave Copeland and me spent a bit of time talking about the proper role of comments in Ruby. He wrote a nice piece defending the use of code level comments back then. We disagree. You should go read his piece before getting to my argument. And after you read that you should go buy his new book.

I’ll wait…

Welcome back.

Dave and I wholeheartedly agree on API Documentation. If you are writing a reusable library—even if only you and your coworkers will ever use it—I find it irresponsible to neglect API level documentation.

We semi-agree on what he calls “Why” comments: comments that explain something about the reasons why a piece of code is the way it is. Sometimes there truly is some odd, external constraint that needs to be explained… and it needs to be explained right there in the code. However, I think that explanatory comments are overused and most times indicate a failure of communications that could better be solved by clear tests, improved naming, and good commit messages. I hope to write about this more later.

What I’m writing about today—and where we disagree—is on the used of descriptive comments: comments that explain what a piece of code is doing.

Problem: Your code needs to be commented to be understood. Solution: Add comments - Now you have 2 problems :)

Dave’s argument for explanatory comments is that even the best of us occasionally write code that is convoluted and difficult to understand. I know that, despite my best efforts, I’m responsible for much of the less than clear code that prompted the discussion on comments in the first place.

Here is the example Dave used of convoluted code you could come across in a production app that would benefit from comments:

Original
1
2
3
4
5
6
7
8
9
10
11
12
13
14
def isqrt(square)
  square = square.to_i
  return 0 if square == 0
  raise RangeError if square < 0

  n = iter(1, square)
  n1 = iter(n, square)
  n1, n = iter(n1, square), n1 until n1 >= n - 1
  n1 = n1 - 1 until n1*n1 <= square
  return n1
end
def iter(n, square)
  (n + (square / n)) / 2
end

No argument from me. That code is impenetrable. As I write this I still don’t completely understand what it does. Dave suggests adding comments so that the next poor bastard who comes along has an easier time of figuring it out. Here’s his commented version:

Dave’s Version with Comments
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
def isqrt(square)
  # just in case we don't get an int
  square = square.to_i
  return 0 if square == 0
  raise RangeError if square < 0

  # make our initial guesses, which are initially
  # too high (intentionally; see next)
  n = iter(1, square)
  n1 = iter(n, square)

  # Refine our guesses until we get close enough
  n1, n = iter(n1, square), n1 until n1 >= n - 1
  # We're close enough when we're JUST under square
  n1 = n1 - 1 until n1*n1 <= square
  return n1
end
def iter(n, square)
  (n + (square / n)) / 2
end

And that is an improvement. However, even with the comments I still don’t fully understand what this method is doing. That’s the problem. Dave went to the trouble of figuring out this really tricky piece of code. He even tried to help me— the aforementioned poor bastard reading this code at a later date—and I still don’t get it.

The problem is that despite the helpful comments (and they are helpful) the method is still pretty much incomprehensible.

The hard part of improving a difficult piece of code is understanding it, not making the changes to improve it. After you’ve managed to understand the code you can improve it in two ways: add comments that describe what it is doing or refactor the code to make it easier to understand. Dave did all the hard work of figuring out the isqrt method. But it is still a difficult method to follow.

Let’s see if we can make the code speak for itself.

My first change was the name of the method from isqrt to integer_square_root_of. There is no reason to drop vowels like we’re writing C in the 70’s. Storage is cheap. Editors autocomplete. Don’t be afraid of descriptive variable and method names. I’m not sure if the “i” in isqrt was supposed to mean integer because the method returns an integer approximation of the square root, or iterative because the method uses an iterative algorithm to determine the square root. I went with “integer” so that the method signature now describes exactly what it does: give you the integer square root of a square.

My Version
1
def integer_square_root_of(square)

My next change was to drop the comment just in case we don't get an int. This comment doesn’t really hurt, but it also doesn’t add anything that isn’t already immediately obvious. Furthermore, there should be a test that shows that the method behaves as expected when receiving a non integer parameter.

Test
1
2
3
4
test "integer_square_root_of should convert non int squares to int" do
  assert_equal 3, integer_square_root_of(9.9)
  assert_equal 3, integer_square_root_of('9')
end

Not hurting isn’t a good enough reason to add a comment to your code. There must be a compelling reason to add a comment. Since this comment doesn’t meet that standard we’re going to go ahead and remove it.

Next up are the special cases: when the square is 0 we short circuit and return 0. When the square is negative we raise a RangeError because there is no such real number square root of a negative number and our method would go into an infinite loop if we didn’t raise an exception. In the original version they were written like this:

Original
1
2
3
  square = square.to_i
  return 0 if square == 0
  raise RangeError if square < 0

The first thing I did was add a blank line to separate them from line 2 (line 1 above as I haven’t figured out how to specify line numbers in Octopress) where we ensure that square is an integer. Blank lines inside of a method definition are greatly underutilized as a tool for grouping related code and improving readability. Think of your application's source code as a book. Classes are chapters. Methods are sections. Each line of code is a sentence. And using blank lines lets us group our “sentences” (lines of code) into paragraphs.

Next I changed the checks for the special cases from two independent lines with postfix if statements to a combined if / elsif structure. Functionally, this is equivalent to the original version. However, by choosing to group them together in one logical block it further serves to emphasize that these two lines are related and draws further attention to the conditionals.

I also add a message to the RangeError that is raised if the square is a negative number. This is an example of what I call executable comments. The body of the message explains why we’re raising the exception—much as a comment would—but because it is part of the executable code it is far less likely to fall out of sync as the code changes.

This yields the following snippet:

My Version
1
2
3
4
5
6
7
  square = square.to_i

  if square == 0
    return 0
  elsif square < 0
    raise RangeError, "square must be non-negative. No negative number has a real square root"
  end

Continuing on, variable names like n1 and n2 don’t tell us anything, so I changed them to upper_guess and lower_guess. These names are far from perfect, but they at least suggest that as we continue to iterate these two variables will converge on the square root.

My Version
1
2
  upper_guess = next_guess(1, square)
  lower_guess = next_guess(upper_guess, square)

I also renamed the method iter to next_guess as that is what it does, give us our next guess.

My Version
1
2
3
def next_guess(n, square)
  (n + (square / n)) / 2
end

Now for the main work of the method. First, we repeatedly refine our guesses. Then, when we’re close enough, we decrement the guess until we get to our answer.

Dave’s Version with Comments
1
2
3
4
5
6
  # Refine our guesses until we get close enough
  n1, n = iter(n1, square), n1 until n1 >= n - 1
  # We're close enough when we're JUST under square
  n1 = n1 - 1 until n1*n1 <= square
  return n1
end

First, I again introduce whitespace to separate logically distinct steps.

The next thing I do is unpack the loop statements from one-liners to blocks. The ability to use postfix loop operators is a good tool for brevity in Ruby. However, a double variable assignment and a loop operator is simply too much for one line. By splitting the loop condition from the variable assignment I allow the reader to focus on each part independently.

Finally, I switch from to while from until for the looping logic. while says “check if this is true then execute the statement and loop” until says “check if this statement is false, and then execute the statement and loop.” This may be a matter of preference, but I find positive logic easier to understand then negative logic.

My Version
1
2
3
4
5
6
7
8
9
10
  while lower_guess < (upper_guess - 1) do
    lower_guess, upper_guess = next_guess(lower_guess, square), lower_guess
  end

  while lower_guess * lower_guess > square do
    lower_guess = lower_guess - 1
  end

  return lower_guess
end

Now, my version is complete:

My Version
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
def integer_square_root_of(square)
  square = square.to_i

  if square == 0
    return 0
  elsif square < 0
    raise RangeError, "square must be non-negative. No negative number has a real square root"
  end

  upper_guess = next_guess(1, square)
  lower_guess = next_guess(upper_guess, square)

  while lower_guess < (upper_guess - 1) do
    lower_guess, upper_guess = next_guess(lower_guess, square), lower_guess
  end

  while lower_guess * lower_guess > square do
    lower_guess = lower_guess - 1
  end

  return lower_guess
end

def next_guess(n, square)
  (n + (square / n)) / 2
end

This post took quite some time to write because explaining code takes a long time. Thankfully, writing code doesn’t. I think it only took me about 15 minutes to read, understand, and rewrite the isqrt method. My bet is that it would have taken about the same amount of time to read, understand, and comment it.

Is this clearer than Dave’s commented version? I obviously think it is, but it still a bit unclear. Perhaps a comment or two would help. My argument isn’t that all comments are unnecessary. It is that the vast majority of comments are unnecessary and that time spent writing comments can be better spent writing clearer code.

What do you think?