# Refactoring question

Page 1 of 1

## 2 Replies - 994 Views - Last Post: 23 January 2014 - 11:04 PMRate Topic: //<![CDATA[ rating = new ipb.rating( 'topic_rate_', { url: 'http://www.dreamincode.net/forums/index.php?app=forums&module=ajax&section=topics&do=rateTopic&t=338814&amp;s=b9742bd8351785b1155cd101cc4e885b&md5check=' + ipb.vars['secure_hash'], cur_rating: 0, rated: 0, allow_rate: 0, multi_rate: 1, show_rate_text: true } ); //]]>

### #1 heaphyg

Reputation: 4
• Posts: 85
• Joined: 30-August 13

# Refactoring question

Posted 23 January 2014 - 07:49 PM

This program checks to see if a credit card number is valid or not. Should I look into splitting my check_card method into multiple methods or is it fine in it's current state. I'm trying to consider the "single responsibility" principal but I guess I'm a little unconfident about what that means exactly. The method does achieve a single task (checking the validation of a number) however, it takes several steps to achieve this end. Should I divide each step into it's own method and figure out how to link those methods together so that a single method call on the CreditcCard object runs the whole program? Thanks for any guidance.

```class CreditCard
def initialize(card_number)
card_number = card_number.to_s
raise ArgumentError.new("Please input a 16 digit number.") if card_number.length != 16  # clean up the input data

@number_array = card_number.split("").map { |x| x.to_i }                  # split our card number into an array of 16 integers (the meat)
end

def check_card                                                              # starting from the penultimate digit in @number_array double every other digit until you reach the first digit
new_array = []
n = -2
8.times do
new_array << @number_array[n]                                           # create a new_array that has every other value from penultimate to beginning
n -=2
end
doubled_array = new_array.collect { |n| n*2 }                             # creates a new array with the values from new_array doubled
split_array = doubled_array.join("").split("").map { |x| x.to_i }         # this will split all of our multi digit numbers into 2 elements

# create an array with the other values from our origional array of 16 numbers
other_digits_array = []
n = -1
8.times do
other_digits_array << @number_array[n]
n -=2
end

master_array = split_array + other_digits_array
num = master_array.inject { |acc, n| acc + n } # sum up all the elements and assign the value to num
if num % 10 == 0 # i should probably create a method here that calls the truth
return true
else
return false
end
end
end

card_number = CreditCard.new(4563960122001999)
puts card_number.check_card

```

Is This A Good Question/Topic? 0

## Replies To: Refactoring question

### #2 Lemur

• Pragmatism over Dogma

Reputation: 1439
• Posts: 3,609
• Joined: 28-November 09

## Re: Refactoring question

Posted 23 January 2014 - 10:23 PM

A tinge verbose on that one. Let's clean it up, shall we?

Now, to explain a refactoring here:
```class CreditCard
def initialize(card_number)
@number_array = card_number.to_s.tap { |s| # Change the card number into a string, tap the results
# Since tap ALWAYS returns self, we're just taking its value from after the to_s call,
raise 'Please input a 16 digit number' unless s.length == 16 # doing some stuff with it,
}.chars.map(&:to_i) # ...and letting it go on its' merry way.
# chars returns an array of characters,
# which we map with the method to_i.
end

def check_card
@number_array # Take the number array
.map                   # Map over each digit
.with_index { |x, i|   # ..with its' index
i.even? ? x * 2 : x  # If the index is even, return the value doubled, else leave it be
}.reduce(:+) % 10 == 0 # Reduce the array using the + function, and mod 10 it.

# Since Ruby implies returns, it'll return whether or not its' mod
# 10 is 0. No need to be explicit on something like that.
end
end

puts CreditCard.new(4563960122001999).check_card # => false
puts CreditCard.new(4563960122001992).check_card # => true

```

Without comments, that's 13 lines of (admittedly slightly dense) code from the original 40. Ruby lets you be incredibly concise with things if you treat it like Ruby instead of like a C language.

Challenge

Now here's a followup question: How many valid credit card numbers are there from 0 to 9999_9999_9999_9999?

Oh, and a conditional to that, you have to pass credit card digits that are less than 16 digits like 0, and make them run. Any more digits and it fails.

0 is valid
0000_0000_0000_0000 is valid (ruby ignores underscores and assumes you're making the number more readable.)
0000_0000_0000_00001 is not valid

Bonus

You'll notice real quickly that takes a heck of a long time. Multi thread the process by breaking up the process into smaller pieces.

This post has been edited by Lemur: 23 January 2014 - 10:39 PM

### #3 heaphyg

Reputation: 4
• Posts: 85
• Joined: 30-August 13

## Re: Refactoring question

Posted 23 January 2014 - 11:04 PM

Wow...that's some cool stuff and thanks for showing me some new methods. I really appreciate it.