# Validating IBAN in Ruby

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

• This does not look like IBAN at all. – steenslag Apr 2 '18 at 21:54

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

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

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

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


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

end

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

def checksum
# TODO
end

def valid?
# TODO
end

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