SilentGhost: dis.dis
does demonstrate underlying conceptual / executional complexity. after all, the OP complained about the original replacement chain being too ‘clumsy’, not too ‘slow’.
i recommend against using regular expressions where not inevitable; they just add conceptual overhead and a speed penalty otherwise. to use translate()
here is IMHO just the wrong tool, and nowhere as conceptually simple and generic as the original replacement chain.
so you say tamaytoes, and i say tomahtoes: the original solution is quite good in terms of clarity and genericity. it is not clumsy at all. in order to make it a little denser and more parametrized, consider changing it to
phone_nr_translations = [
( ' ', '', ),
( '(', '', ),
( ')', '', ), ]
def sanitize_phone_nr( phone_nr ):
R = phone_nr
for probe, replacement in phone_nr_translations:
R = R.replace( probe, replacement )
return R
in this special application, of course, what you really want to do is just cancelling out any unwanted characters, so you can simplify this:
probes = ' ()'
def sanitize_phone_nr( phone_nr ):
R = phone_nr
for probe in probes:
R = R.replace( probe, '' )
return R
coming to think of it, it is not quite clear to me why you want to turn a phone nr into an integer—that is simply the wrong data type. this can be demonstrated by the fact that at least in mobile nets, +
and #
and maybe more are valid characters in a dial string (dial, string—see?).
but apart from that, sanitizing a user input phone nr to get out a normalized and safe representation is a very, very valid concern—only i feel that your methodology is too specific. why not re-write the sanitizing method to something very generic without becoming more complex? after all, how can you be sure your users never input other deviant characters in that web form field?
so what you want is really not to dis-allow specific characters (there are about a hundred thousand defined codepoints in unicode 5.1, so how do catch up with those?), but to allow those very characters that are deemed legal in dial strings. and you can do that with a regular expression...
from re import compile as _new_regex
illegal_phone_nr_chrs_re = _new_regex( r"[^0-9#+]" )
def sanitize_phone_nr( phone_nr ):
return illegal_phone_nr_chrs_re.sub( '', phone_nr )
...or with a set:
legal_phone_nr_chrs = set( '0123456789#+' )
def sanitize_phone_nr( phone_nr ):
return ''.join(
chr for chr in phone_nr
if chr in legal_phone_nr_chrs )
that last stanza could well be written on a single line. the disadvantage of this solution would be that you iterate over the input characters from within Python, not making use of the potentially speeder C traversal as offered by str.replace()
or even a regular expression. however, performance would in any case be dependent on the expected usage pattern (i am sure you truncate your phone nrs first thing, right? so those would be many small strings to be processed, not few big ones).
notice a few points here: i strive for clarity, which is why i try to avoid over-using abbreviations. chr
for character
, nr
for number
and R
for the return value (more likely to be, ugh, retval
where used in the standard library) are in my style book. programming is about getting things understood and done, not about programmers writing code that approaches the spatial efficiency of gzip. now look, the last solution does fairly much what the OP managed to get done (and more), in...
legal_phone_nr_chrs = set( '0123456789#+' )
def sanitize_phone_nr( phone_nr ): return ''.join( chr for chr in phone_nr if chr in legal_phone_nr_chrs )
...two lines of code if need be, whereas the OP’s code...
class Phone():
def __init__ ( self, input ): self.phone = self._sanitize( input )
def __str__ ( self ): return self.phone
def _sanitize ( self, input ): return input.replace( ' ', '' ).replace( '(', '' ).replace( ')', '' )
...can hardly be compressed below four lines. see what additional baggage that strictly-OOP solution gives you? i believe it can be left out of the picture most of the time.