Validating IBAN in Ruby

Question

Looking for tips on how to improve my code and areas that I can better follow Object Orientated principles. Is this class structure ok? How is the code overall?

The problem: There are two IBAN standards: IBAN-10 and IBAN-13. IBAN-10 is made up of 9 digits plus a check digit. IBAN-13 is made up of 12 digits plus a check digit. Spaces and hyphens may be included in a code but are not significant.

The check for IBAN-10 is calculated by multiplying each digit by its position, summing these products together and taking the modulus 11 of the result. Substituting X if the result is 10.

The check digit for IBAN-13 is calculated by multiplying each digit alternately by 1 or 3, summing these products together and taking the modulus 10 of the result and subtracting this value from 10. Then with this result take the modulus of ten so as to reduct the number to a single digit.

Return true if it is a valid IBAN-10/IBAN-13

class IBAN

  def initialize(number)
    puts "number: #{number}"

    @number = number
    @number.gsub!(/[^0-9]/i, '')
    return @number
  end

  def number
    @number
  end

  # # check IBAN length and if valid proceed to process IBAN type
  def check_number_format
    iban_length = self.number.length

    unless iban_length == 10 || iban_length == 13
      puts "Not a valid IBAN - incorrect length"
      return false
    else
      self.validate_iban_type
    end
  end

  def validate_iban_type
    case self.number.length
    when 10
      @iban_ten = IBANTEN.new(self.number)
      @iban_ten.process_ten_digit_iban
    when 13
      @iban_thirteen = IBANTHIRTEEN.new(self.number)
      @iban_thirteen.process_thirteen_digit_iban
    else
      return false
    end
  end
end


class IBANTEN

  def initialize(iban_number)
    @iban_number = iban_number
  end

  def iban_number
    @iban_number
  end

  def process_ten_digit_iban
    number_split = self.iban_number.split('')
    final_digit = number_split[-1]
    running_total = 0

    number_split[0, (number_split.length - 1)].each_with_index do |num, index|
      # multiply each digit by its position
      sum = num.to_i * (index.to_i + 1)
      running_total += sum.to_i
    end
    # get the modulus of the running total if it exuals 10 replace it with X otherwise return the result
    digit_sum_total = running_total % 11
    calculated_result = digit_sum_total == 10 ? "X" : digit_sum_total

    if calculated_result == final_digit.to_i
      puts "Valid IBAN-10 Number"
      return true
    else
      puts "Invalid IBAN-10 Number"
      return false
    end
  end

end


class IBANTHIRTEEN

  def initialize(iban_number)
    @iban_number = iban_number
  end

  def iban_number
    @iban_number
  end

  def process_thirteen_digit_iban
    number_split = self.iban_number.split('')
    final_digit = number_split[-1]
    running_total = 0

    number_split[0, (number_split.length - 1)].each_with_index do |num, index|
      # if the remainder equals zero multiply by 1 otherwise multiplu by 3
      sum = (index % 2 == 0 ) ? num.to_i * 1 : num.to_i * 3
      running_total += sum.to_i
    end
    # modulo 10 of the result and subtracting this value from 10, and then taking the modulo 10 of the result again to produce a single digit
    calculated_result = ( (10 - (running_total % 10) ) % 10)

    if calculated_result == final_digit.to_i
      puts "Valid IBAN-13 Number"
      return true
    else
      puts "Invalid IBAN-13 Number"
      return false
    end
  end
end


@iban_v13_valid = IBAN.new("978 0 471 48648 0")
@iban_v13_valid.check_number_format

@iban_v13_invalid = IBAN.new("9780470059021")
@iban_v13_valid.check_number_format

@iban_v10_valid = IBAN.new("0 471 60695 2")
@iban_v10_valid.check_number_format

@iban_v10_invalid = IBAN.new("0-470-84525-6")
@iban_v10_invalid.check_number_format


@iban_invalid_length = IBAN.new("0-470-84525-618423")
@iban_invalid_length.check_number_format

Answer 1

Assuming you prefer using 3 different classes;

• Use Rubocop gem.
  • Prefer guard clauses.
  • Prefer single quotes over double.
  • Prefer local variables when instance variables are not needed.
  • No need for explicit returns.
• Understand the difference between gsub! and gsub.
• Don't need to write self implicitly.
• Prefer attr_ rather than writing methods.
• Personally, not a big fan of using initializers to modify data. If you do, keep it minimal.
• Use shorter methods. Read single responsibility principle (SRP).
• Use simpler variable names.
• Didn't refactor long methods as it can depend on personal references. However:
  • You can extract both #process_ten_digit_iban and #process_thirteen_digit_iban as they are very similar.
  • You can extract puts valid/invalid in if conditions to keep your code dry.

---

class IBAN
  REQUIRED_LENGTH = [10, 13].freeze

  attr_reader :number

  def initialize(number)
    @number = number.gsub(/[^0-9]/i, '')
  end

  def validate_iban_type
    return if check_number_format?

    case number.length
    when 10
      IBANTEN.new(number).process_ten_digit_iban
    when 13
      IBANTHIRTEEN.new(number).process_thirteen_digit_iban
    else
      false
    end
  end

  # check IBAN length and if valid proceed to process IBAN type
  def check_number_format?
    iban_length = number.length
    raise 'Incorrect length' unless REQUIRED_LENGTH.include?(iban_length)
  end
end

class IBANTEN
  attr_reader :iban_number

  def initialize(iban_number)
    @iban_number = iban_number
  end

  def process_ten_digit_iban
    numbers_array = iban_number.split('')
    total = 0

    numbers_array[0, (numbers_array.length - 1)].each_with_index do |num, index|
      # multiply each digit by its position
      sum = num.to_i * (index.to_i + 1)
      total += sum
    end

    # get the modulus of the running total if it exuals 10 replace it with X
    # otherwise return the result
    digit_sum_total = total % 11
    result = digit_sum_total == 10 ? 'X' : digit_sum_total

    if result == numbers_array[-1].to_i
      puts 'Valid IBAN-10 Number'
      true
    else
      puts 'Invalid IBAN-10 Number'
      false
    end
  end
end

class IBANTHIRTEEN
  attr_reader :iban_number

  def initialize(iban_number)
    @iban_number = iban_number
  end

  def process_thirteen_digit_iban
    numbers_array = iban_number.split('')
    total = 0

    numbers_array[0, (numbers_array.length - 1)].each_with_index do |num, index|
      # if the remainder equals zero multiply by 1 otherwise multiply by 3
      sum = index.even? ? num.to_i * 1 : num.to_i * 3
      total += sum
    end

    # modulo 10 of the result and subtracting this value from 10, and then
    # taking the modulo 10 of the result again to produce a single digit
    result = ((10 - (total % 10)) % 10)

    if result == numbers_array[-1].to_i
      puts 'Valid IBAN-13 Number'
      true
    else
      puts 'Invalid IBAN-13 Number'
      false
    end
  end
end

iban_v13_valid = IBAN.new('978 0 471 48648 0')
iban_v13_valid.validate_iban_type

iban_v13_invalid = IBAN.new('9780470059021')
iban_v13_invalid.validate_iban_type

iban_v10_valid = IBAN.new('0 471 60695 2')
iban_v10_valid.validate_iban_type

iban_v10_invalid = IBAN.new('0-470-84525-6')
iban_v10_invalid.validate_iban_type

iban_invalid_length = IBAN.new('0-470-84525-618423')
iban_invalid_length.validate_iban_type

Answer 2

I would look to implement a structure with separate implementations for the three cases:

1. IBAN13
2. IBAN10
3. Neither of the above

You can provide that with something along these lines, in which IBAN#new is overridden to detect the number of integers provided, and to initialise the correct object to process it.

The real trick here is to provide the Invalid module, which responds to everything that you expect the Thirteen and Ten objects to respond to, in an implementation similar to a Nil Object pattern – in this case an Invalid Object.

I've omitted the implementation of the #valid? etc classes, but would point you to the Lisbn gem in which we have implemented an extremely fast algorithm https://github.com/ragalie/lisbn/blob/master/lib/lisbn/lisbn.rb#L111. Benchmarked against Enumerable-based methods, which often look quite elegant, this algorithm is incredibly fast – 40x faster than the same logic implemented in some ISBN gems.

Here's a code outline:

module IBAN
  def self.new(string)
    numeric = string.gsub(/[^0-9]/,"")
    case numeric.size
    when 13
      then Thirteen.new(string)
    when 10
      then Ten.new(string)
    else
      Invalid
    end
  end

  class Thirteen
    def initialize(number)
      @number = number
    end

    def checksum
      # TODO
    end

    def valid?
      # TODO
    end

    attr_reader :number
  end

  class Ten
    def initialize(number)
      @number = number
    end

    def checksum
      # TODO
    end

    def valid?
      # TODO
    end

    attr_reader :number
  end

  module Invalid
    def self.valid?
      false
    end

    def self.checksum
      nil
    end
  end
end

ps. You might prefer to implement Invalid as a singleton instead of as a module.

    ...
    else
      Invalid.instance
    end
    ...

  class Invalid
    include Singleton

    def valid?
      false
    end

    def checksum
      nil
    end
  end
end