Ruby Mod 11 implementation

Question

I wrote this Ruby Mod 11 (perhaps it's a bit generous to call it that) implementation to validate Brazilian CPF documents, which use a Mod 11 algorithm. CPF format is a 9 digit number followed by two checksums. It's often presented in this format 012.345.678-90. Feedback is welcome.

class CPF
 attr_reader :digits
 LENGTH = 11
 def self.valid?(number)
 new(number).valid?
 end
 def self.mask(number)
 new(number).mask
 end
 def initialize(number)
 @digits = number.to_s.gsub(/\D/, '').each_char.map(&:to_i)
 end
 def valid?
 correct_length? && !black_listed? && checksums_match?
 end
 # The `black_listed?` method checks common number patterns.
 #
 # It tests to see if the provided CPF is the same digit repeated n times.
 #
 def black_listed?
 digits.join =~ /^12345678909|(\d)1円{10}$/
 end
 # A valid CPF must be 11 digits long.
 #
 def correct_length?
 digits.size == LENGTH
 end
 # A CPF is only valid if the first and second checksum digits are what they should be.
 #
 def checksums_match?
 first_checksum_matches? && second_checksum_matches?
 end
 # This returns the CPF is a human readable format.
 #
 # CPF.mask("01234567890")
 # => "012.345.678-90"
 #
 def mask
 [digits.first(9).each_slice(3).map(&:join).join("."), digits.last(2).join].join('-') if valid?
 end
private
 def first_checksum_matches?
 checksum_one == digits.at(9)
 end
 def second_checksum_matches?
 checksum_two == digits.at(10)
 end
 def checksum_one
 digit = sum_at(10)
 digit = calculate(digit)
 digit = 0 if digit > 9
 digit
 end
 def checksum_two
 digit = sum_at(11) + (2 * checksum_one)
 digit = calculate(digit)
 digit = 0 if digit > 9
 digit
 end
 def sum_at(position)
 digits.slice(0..8).collect.with_index { |n, i| n * (position - i) }.reduce(:+)
 end
 def calculate(digit)
 LENGTH - (digit % LENGTH)
 end
end

Tests

require 'test_helper'
class CpfTest < ActiveSupport::TestCase
 test 'black listed numbers are not' do
 black_list = %w(00000000000 11111111111 22222222222 33333333333 44444444444
 55555555555 66666666666 77777777777 88888888888 99999999999)
 black_list.each do |number|
 assert_invalid number
 end
 end
 test 'nil is not a valid CPF' do
 assert_invalid nil
 end
 test 'blank is not a valid CPF' do
 assert_invalid ''
 end
 test 'valid CPF' do
 assert_valid '01234567890'
 end
 test 'masked valid CPF' do
 assert_valid '012.345.678-90'
 end
 test 'mask returns a masked CPF when CPF is valid' do
 assert_equal '012.345.678-90', CPF.mask('01234567890')
 end
 test 'mask returns nil when CPF is not valid' do
 assert_nil CPF.mask('0123456789')
 end
private
 def assert_invalid(number)
 refute CPF.valid?(number), "Expected #{number || 'nil'} to be an invalid CPF"
 end
 def assert_valid(number)
 assert CPF.valid?(number), "Expected #{number} to be a valid CPF"
 end
end

• DarkDust: I was wondering why you've called the method that returns a string mask. I think the name is non-obvious, at least to me (might be obvious in your context, though). Why not simply use the default to_s instead?
• Mohamad: @DarkDust good point. I called it mask because such numbers are usually stored without any formatting. For for display, however, after retrieval, one may want to mask the number. I guess that shouldn't stop me from using a to_s.
• Mohamad: @DarkDust also, it's to allow me to use the class method CPF.mask("01234567890") #=> "012.345.678-90"
• Jamal: Although the updated code was added before the first answer, I've removed it since the first version has already been reviewed. Keeping the updated code may cause some confusion.

Answer 1

Your code looks pretty good, but there is a no-no:

def checksum_one
 digit = sum_at(10)
 digit = calculate(digit)
 digit = 0 if digit > 9
 digit
end

Note thatdigit my have up to three different values in just four lines of code. This makes the code harder to follow. As a rule of thumb, use different variable names. That's how I'd write it:

def checksum_one
 digit = calculate(sum_at(10))
 digit > 9 ? 0 : digit
end

• Mohamad: Thanks! As you might have noticed your previous code reviews have taught me to use more functional programming. Thanks for that.
• tokland: You're welcome! Yeah, I noticed some functional bits in your code, they look nice :)

Answer 2

Tokland already gave a fine answer. I'll just offer a few more suggestions here and there.

• The underscore in black_listed implies that it's two words in regular English. However, "blacklist" and "blacklisted" are in single words each.

• The mask method could perhaps be more "direct" with a regular expression:

  def mask
   digits.join.sub(/(\d{3})(\d{3})(\d{3})(\d{2})/, '1円.2円.3円-4円') if valid?
  end
  

  Incidentally, I think simply overriding to_s would be better than having a method named "mask". It's not entirely clear what "masking a CPF" would mean.

• I'd perhaps add some methods to pull the "payload" digits and the checksum digits, e.g.

  def checksums
   digits[-2, 2]
  end
  def payload
   digits[0, 9]
  end
  

  This could perhaps make the checksum-checking simpler, like so (note the use of Array#rotate)

  def calculate_checksum(digits)
   sum = digits.map.with_index { |digit, i| digit * (i+1) }.inject(&:+)
   sum % 11 % 10
  end
  def checksums_match?
   checksums == [ calculate_checksum(payload), calculate_checksum(payload.rotate) ]
  end
  

  This avoid having two very similar methods to check either checksum individually.

• Mohamad: Thanks for the review. Great feedback. Code reviews always seem to make me feel perpetually bad at programming, but teach me a heck of a lot!