Lua program to clean up phone numbers

Question

How idiomatic is my code? Since Lua does not have classical inheritance such as in OO languages. Feedback on the test and implementation are both appreciated/welcomed.

The rules are:

If the phone number is less than 10 digits assume that it is bad number
If the phone number is 10 digits assume that it is good
If the phone number is 11 digits and the first number is 1, trim the 1 and use the first 10 digits
If the phone number is 11 digits and the first number is not 1, then it is a bad number
If the phone number is more than 11 digits assume that it is a bad number

local PhoneNumber = {}

function PhoneNumber:new(no_as_string)
  self.__index = self
  local n = "0000000000"
  n = no_as_string:gsub("[^0-9]", "")
  if (n:len() == 11 and n:sub(1, 1) == "1") then 
    n = n:sub(2, 11)
  else 
    if (n:len() > 10 or n:len() < 10) then
      n = "0000000000"
    end
  end
  return setmetatable({ number = n }, self)
end

function PhoneNumber:areaCode(symbol)
  return self.number:sub(1, 3)
end

function PhoneNumber:toString(symbol)
  return "("..self.number:sub(1, 3)..") "..self.number:sub(4, 6).."-"..self.number:sub(7, 10)
end

return PhoneNumber

Here is the test:

local PhoneNumber = require('phone-number')

describe("PhoneNumber()", function() 
  it("cleans the number (123) 456-7890", function() 
    local phone = PhoneNumber:new("(123) 456-7890")
    assert.are.equals(phone.number,"1234567890")
  end)

  it("cleans numbers with dots", function()
    local phone = PhoneNumber:new("123.456.7890")
    assert.are.equals(phone.number,"1234567890")
  end)

  it("valid when 11 digits and first digit is 1", function()
    local phone = PhoneNumber:new("11234567890")
    assert.are.equals(phone.number,"1234567890")
  end)

  it("invalid when 11 digits", function()
    local phone = PhoneNumber:new("21234567890")
    assert.are.equals(phone.number,"0000000000")
  end)

  it("invalid when 9 digits", function()
    local phone = PhoneNumber:new("123456789")
    assert.are.equals(phone.number,"0000000000")
  end)

  it("has an area code", function()
    local phone = PhoneNumber:new("1234567890")
    assert.are.equals(phone:areaCode(),"123")
  end)

  it("formats a number", function()
    local phone = PhoneNumber:new("1234567890")
    assert.are.equals(phone:toString(),"(123) 456-7890")
  end)
end)

hjpotter92 · Accepted Answer · 2014-11-23 20:30:25Z

You've a lot of unnecessary code. First off, as janos stated:

local n = "0000000000"
n = no_as_string:gsub("[^0-9]", "")
The initialization of n is pointless here.

You can instead use just the lua pattern to match your input to desired result:

function PhoneNumber:new( Number )
    local n = string.gsub( Number, "%D", "" )
    n = n:match "^1?(%d%d%d%d%d%d%d%d%d%d)$" or "0000000000"
    return setmetatable({ number = n, __index = self }, self)
end

Notice that I've changed the input argument to Number and later use string.gsub( Number.... This way; you've the advantage of inputting direct numbers and provide flexibility.

The pattern

"^1?(%d%d%d%d%d%d%d%d%d%d)$"

is self-explanatory. In lua, if string.match failed to find any matched group; it returns nil. I've used this property to check for bad numbers.

Next thing I'd suggest that you do is store the number as an integer instead of string:

return setmetatable({ number = tonumber(n), __index = self }, self)

This is just because integers take less memory than strings. For retrieving area code etc., use mathematical operations:

function PhoneNumber:areaCode()  -- what was symbol parameter for?
    return math.floor( self.number / 10^7 )
end

Lastly, lua provides a meta-method: __tostring for conversion to strings from other types. Use it (It is the reason why I was able to use string.gsub above):

function PhoneNumber:__tostring()
    local f, n = math.floor, self.number
    local sFormat, iPart1, iPart2 = "(%d) %d-%d", f( n / 10^7 ), f( n / 10^3 % 10^4 )
    return sFormat:format( self:areaCode(), iPart1, iPart2 )
end

Now, instead of calling PhoneObject:toString(), you can do:

tostring( PhoneObject )

So many concepts explained. -I had been wondering how to use tostring in lua - string.gsub vs string:gsub - also the regex is sweet. -I did not even know abt the format function :) — aarti
– aarti, Commented Nov 25, 2014 at 19:42

janos · Accepted Answer · 2014-11-23 12:32:30Z

  local n = "0000000000"
  n = no_as_string:gsub("[^0-9]", "")

The initialization of n is pointless here. You should combine these two lines.

  if (n:len() == 11 and n:sub(1, 1) == "1") then 
    n = n:sub(2, 11)
  else 
    if (n:len() > 10 or n:len() < 10) then
      n = "0000000000"
    end
  end

Using an elseif for the second if would reduce a level of nesting and be shorter and simpler.

    if (n:len() > 10 or n:len() < 10) then

Instead of checking if len is bigger than 10 or smaller than 10, it would be simpler to check equality with 10. That would also eliminate one unnecessary call to len(). That is:

    if (n:len() != 10) then

assert.are.equals(phone.number,"1234567890")

In unit testing, the convention in many languages is to put the expected value first, and the actual value second in assert statements. So I suggest to flip the arguments.

Stack Exchange Network

Lua program to clean up phone numbers

2 Answers 2

You must log in to answer this question.

Hot Network Questions

Lua program to clean up phone numbers

2 Answers 2

You must log in to answer this question.

Related

Hot Network Questions