Newbie: Code critique

[kpeters]

In an effort to improve my Transcript coding - does someone have suggestion on how to improve the following function?

TIA,
Kai

Code: Select all

function FormatPhoneNumber pPhoneNumber
  local tmp
  --
  put empty into tmp
  # strip all non-numerical characters from phone number
  repeat with i = 1 to length( pPhoneNumber )
    if char i of pPhoneNumber is an integer then put char i of pPhoneNumber after tmp
  end repeat
  # now use standardized formats
  switch ( length( tmp ))
  case 0
    return ""
    break
  case 7
    return char 1 to 3 of tmp & "-" & char 4 to 7 of tmp
    break
  case 11
    put char 2 to 11 of tmp into tmp -- shorten to 10 & let if fall through!
  case 10
    return "1 (" & char 1 to 3 of tmp & ") " & char 4 to 6 of tmp & "-" & char 7 to 10 of tmp
    break
  default
    return False
  end switch
end FormatPhoneNumber

[xApple]

the "local tmp" line is usless.
It's better to use "--" for comments, to not get confused when you are coding c++
A return statement ends the handler, hence a placing a "break" after a "return" is usless

Code: Select all

function FormatPhoneNumber pPhoneNumber 
  -- strip all non-numerical characters from phone number
  repeat for each char C in pPhoneNumber 
    if C is an integer then put C after tmp
  end repeat
  -- now use standardized formats 
  switch the number of characters of tmp
  case 0
    return empty
  case 7 
    put "-" after char 3 of tmp
    break
  case 11 
    -- shorten to 10 and let if fall through
    delete char 1 of tmp
  case 10
    put "1 (" before tmp
    put ") " after char 3 of tmp
    put "-" after char 9 of tmp
    break
  default 
    return false 
  end switch 
  return tmp
end FormatPhoneNumber

[kpeters]

Thanks for your input - just what I was looking for!

BTW, the line "local tmp" is not useless since I am running my scripts with "Variable checking" set to True.

Also, your code for case 10 is broken as you are changing the length of tmp
but your offsets are still based on the length of the original tmp string.

Why do you prefer to use "the number of characters of s" over "Length( s)"?
Any known execution speed issues?

Kai

[xApple]

Ah ok I didn't know that.. I always have variable checking off.

Yes indeed I forgot to change the index numbers in case 10.

Concerning the length function, no I don't think there is any speed difference.. I just try to have my script most human readable.. same reason for changing "repeat with i = 1 to length(pPhoneNumber)" to "repeat for each char C in pPhoneNumber"

[kray]

Here's an alternative form that is a little shorter, but should work just the same (oh btw, when you declare the local 'tmp', it is automatically empty, so you don't need to 'put empty into tmp'):

Code: Select all

function FormatPhoneNumber pPhoneNumber
  local tmp
  
  # strip all non-numerical characters from phone number
  repeat for each char c in pPhoneNumber
    if c is an integer then put c after tmp
  end repeat
  if tmp is empty then return ""
  
  # now use standardized formats
  if length(tmp) is not among the items of "7,10,11" then return "False"
  if length(tmp) <> 7 then  -- must be 10 or 11
    put char -10 to -1 of tmp into tmp  -- use last 10 digits
    put "1 (" & (char 1 to 3 of tmp) & ") " & (char 4 to 10 of tmp) into tmp
  end if
  put "-" before char -4 of tmp  -- needed for both phone forms
  
  return tmp
end FormatPhoneNumber

[kpeters]

Thanks to both of you again - nice stuff that really helps me to tweak my
code!

Kai