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

• steenslag: This does not look like IBAN at all.

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