# Lua program to clean up phone numbers

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)


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 :) Nov 25 '14 at 19:42
  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.