Skip to content

Instantly share code, notes, and snippets.

@isomorpheric
Last active March 28, 2016 17:00
Show Gist options
  • Save isomorpheric/5421a8645d867f801dfc to your computer and use it in GitHub Desktop.
Save isomorpheric/5421a8645d867f801dfc to your computer and use it in GitHub Desktop.
module Luhn
attr_reader :digs
def self.is_valid?(number)
digs = Math.log10(number).floor.downto(0).map { |i| (number / 10**i) % 10 }
# Iterate through every 2nd number, starting from left. Reverse array to accomplish that.
digs = digs.reverse
f_digs = []
# Multiply every 2nd number by 2. If number is greater or equal to 10, substract 9.
digs.each_with_index do |dig,index|
if ( (index != 0) && (index % 2 != 0))
dig *= 2
if dig >= 10
dig -= 9
end
f_digs << dig
else
f_digs << dig
end
end
# If sum of all numbers is divisible by 10, return true, else return false.
sum = f_digs.inject(:+)
(sum % 10 == 0) ? true : false
end
end
require 'spec_helper'
RSpec.describe Luhn, type: :model do
describe 'is_valid? should work' do
it 'should return true if the number is valid' do
expect(Luhn.is_valid?(4194560385008504)).to be true
end
it 'should return false if the number is not valid' do
expect(Luhn.is_valid?(4194560385008505)).to be false
end
it 'should give the wrong answer if you begin on the left side for valid numbers' do
expect(Luhn.is_valid?(377681478627336)).to be true
end
it 'should give the wrong answer if you begin on the left for invalid numbers' do
expect(Luhn.is_valid?(377681478627337)).to be false
end
end
end
@johnnylaw
Copy link

Good job. Love that you have some tests on it. A couple of things to consider:

  1. What is the effect of having the attr_reader for digs in this case? I don't see any mention of @digs anywhere.
  2. On line 12, one of the two anded conditions always implies the other and can be removed.
  3. I believe each_with_index has been deprecated in Ruby in favor of each.with_index. Same thing; just limited future for the former.

Again, good job. Tomorrow we can talk about a couple of finer points. This is a difficult algorithm to code in a TDD manner, because the "intermediate" results between the input and the output of the valid? query are not part of the public interface and so cannot be tested easily (and most would say should not be tested at all).

If we wanted to do TDD on this, the first tests you wrote would be for one-digit numbers, two-digit numbers and so forth. The code that got written to satisfy those low-length numbers would not involve anything as tricky as Math.log10(number)... etc. That would likely come in later during a refactoring step.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment