CODEWITHSUNDEEP
ruby-on-railsrubyruby-on-rails-3
cadlac
cadlac

Reputation: 3158

Simplifying code for formatting phone numbers

What I'm trying to accomplish is to take a string, and parse it so it is just the numbers. Then when displaying it, I'll use the number_to_phone and they'll all be the same.

So far I have this defined in my model:

  def parse_phone_nums
    self.main_phone = self.main_phone.gsub(/[.\-()\W]/, '') if self.main_phone
    self.alt_phone = self.alt_phone.gsub(/[.\-()\W]/, '') if self.alt_phone
    self.main_phone = "925" + self.main_phone if self.main_phone.length == 7
    self.alt_phone = "925" + self.alt_phone if self.alt_phone.length == 7
  end

And call it in my controller on the create and update actions. I feel as if there is a lot of repetition here and was wondering how one might reduce the code to be as DRY as possible.

Upvotes: 2

Views: 486

Answers (2)

doesterr
doesterr

Reputation: 3965

I'd go with something like this, since it allows for future extension (another area code? detect area codes submitted by the user, etc) and is easily testable.

And since it's easy to forget a matcher for a character that the user might use, I'd use a different approach for extracting the number from the given String.

def parse_phone_nums
  self.main_phone = parse_phone_number(self.main_phone) if self.main_phone
  self.alt_phone  = parse_phone_number(self.alt_phone)  if self.alt_phone
end

private

def parse_phone_number(string)
  number = extract_number(string)
  prefix_area_code(number)
end

def extract_number(string)
  characters = string.split("")
  characters.reduce("") { |memo, char| "#{memo}#{char}" if numeric?(char) }
end

def prefix_area_code(number)
  prefix = number.length == 7 ? "925" : ""
  "#{prefix}#{number}"
end

def numeric?(string)
  Float(string) != nil rescue false
end

Ideally I'd extract all these private methods into a own class, let's say a PhoneNumberParser.

If you think def_phone_nums is not DRY enough, or if you have many more phone numbers, this should work:

def parse_phone_nums
  phones = %w[main alt]
  phones.each do |phone|
    current = self.send("#{phone}_phone")
    next unless current

    parsed = parse_phone_number(current)
    self.send("#{phone}_phone=", parsed)
  end
end

Upvotes: 0

Dave Newton
Dave Newton

Reputation: 160201

One possible solution of many, roughly: 

def clean_up_phone(num)
  return unless num
  num = num.gsub(/[.\-()\W]/, '')
  num.length == 7 ? "925#{num}" : num
end

There are a number of ways this method could be used, including automatically on setting, during a callback, and so on.

I'm not sure the regex is what you really want, people enter quite a few things for phone numbers. You may want to process it a bit more before validation, and add the area code after that.

Upvotes: 2

Related Questions