2
\$\begingroup\$

I created my first Ruby program. Even though it works, I wondered whether I could do something different/better?

#!/usr/bin/ruby

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

  def check
    createNumberArray
    doubleSecond
    sumNumbers
    getResult
  end

  def createNumberArray
    @numberArray = @number.scan(/\d/).map { |c| c.to_i }
  end

  def doubleSecond
    puts "Checker: doubling every second number from the right.."
    i = @numberArray.length - 1 
    pos = 0
    while i >= 0
      if pos.modulo(2) == 1
        @numberArray[i] = @numberArray[i] * 2
      end
      i = i - 1
      pos = pos + 1
    end
  end

  def sumNumbers
    puts "Checker: calculating the sum.."
    @sum = 0
    @numberArray.each do |i|
      if i > 9
        @sum = @sum + splittedSum(i) # treat numbers >= 10 individually
                                     # e.g. 12 -> 1 + 2 = 3
      else
        @sum = @sum + i
      end
    end
  end

  def splittedSum(number)
    numberAsString = number.to_s()
    splittedNumbers = numberAsString.scan(/\d/).map { |c| c.to_i }
    splittedSum = 0
    splittedNumbers.each do |n|
      splittedSum = splittedSum + n
    end
    return splittedSum
  end

  def getResult
    if @sum.modulo(10) == 0
      puts "Credit card number is valid"
    else
      puts "Credit card number is not valid"
    end
  end
end

class CreditCard
  def setCardNumber(number)
    @cardNumber = number
  end

  def getCardNumber
    if @cardNumber!= nil
      @cardNumber
    end
  end

  def verify
    puts "Verifying card number now..."
    checker = LuhnyBinChecker.new(@cardNumber)
    checker.check
  end
end

if __FILE__ == $0
  card = CreditCard.new

  puts "Please enter the credit card number to check:"
  gets              # input card number
  input = $_
  if input.match(/[a-z]/i) != nil || input.match(/\d/) == nil
    puts "Please enter only numbers."
    exit
  end

  card.setCardNumber(input)
  puts "You entered: " + card.getCardNumber
  card.verify
end

Edit: here are two input/output examples:

michael@michael-linlap /media/Data/development/private/scripts $ ruby LuhnyBin.rb 
Please enter the credit card number to check:
1111 1111 1111 1111 1111
You entered: 1111 1111 1111 1111 1111
Verifying card number now...
Checker: doubling every second number from the right..
Checker: calculating the sum..
Credit card number is valid
michael@michael-linlap /media/Data/development/private/scripts $ ruby LuhnyBin.rb 
Please enter the credit card number to check:
fooobar1111
Please enter only numbers.
\$\endgroup\$
0

1 Answer 1

4
\$\begingroup\$

Welcome to ruby :), here are a few changes you might want to consider.

#!/usr/bin/ruby

class LuhnyBinChecker

  def initialize(number)
    @number = number
  end

Try to keep your methods pure without i/o, and collect the i/o in some portion that is dedicated to it. It helps later while you refactor the code. It is also nicer to not to rely on member variables too much, and preserve the referential transparency of functions where possible.

  def check
    arr = digits(@number)
    puts "Checker: doubling every second number from the right.."
    arr = doubleSecond(arr)
    puts "Checker: calculating the sum.."
    getResult(sumNumbers(arr))
  end

Always be on the lookout for things you can refactor and reuse.

  def digits(n)
    n.to_s().scan(/\d/).map{ |c| c.to_i }
  end

Two ways of doing the doubleSecond, I prefer the second way, and I think that is a more ruby way. But as a beginner, you might understand the first better. Choose which ever you like.

  def doubleSecond(arr)
    #arr.each_with_index do |item, i|
    #  arr[i] = arr[i] * 2 if (i %2 == 0)
    #end
    arr.zip(([email protected]).to_a()).map do |e|
       e[1] * ((e[0] % 2) + 1)
    end
  end

  # treat numbers >= 10 individually 
  # e.g. 12 -> 1 + 2 = 3
  def sumNumbers(arr)
    arr.map{|i|
        i > 9 ? splittedSum(i) : i
    }.inject(0,:+)
  end

You can think of Array.inject(0,:+) as Array.sum()

  def splittedSum(number)
    digits(number).inject(0,:+)
  end

  def getResult(sum)
    sum.modulo(10) == 0
  end
end

It is nicer to have a guarantee that a CreditCard will always contain a number rather than to check for nil in the getter method.

class CreditCard
  attr_reader :number
  def initialize(number)
    @number = number
  end

  def verify
    (LuhnyBinChecker.new(@number)).check
  end
end

You can use abort if some conditions fail to satisfy.

if __FILE__ == $0
  puts "Please enter the credit card number to check:"
  input = gets()              # input card number
  abort "Please enter only numbers." if input !~/(\d+\s)+/
  card = CreditCard.new(input)
  puts "You entered: " + card.number
  puts "Verifying card number now..."
  if card.verify()
     puts "Credit card number is valid"
  else
     puts "Credit card number is valid"
  end
end
\$\endgroup\$
3
  • \$\begingroup\$ actually, your scan function doesn't work.. LuhnyBin.rb:17:in digits': private method scan' called for 54737527746:Fixnum (NoMethodError) .. how can I fix that? \$\endgroup\$ Commented Jun 17, 2012 at 14:11
  • \$\begingroup\$ Updated. the digits should convert its input to string before scanning (as in your numberAsString). \$\endgroup\$ Commented Jun 17, 2012 at 17:56
  • \$\begingroup\$ Converting the numbers to a String before scanning works. Thanks :) \$\endgroup\$ Commented Jun 17, 2012 at 19:58

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.