1
\$\begingroup\$

Is there anything I can do better here? I tried looking for ways to simplify this with comprehension but could not figure out how and was told it is better to explicitly use for loops.

import re

print("This is a email/phone number parser.")
print("Type in the name of the file you want to parse:\n")
filename = input("> ")

emailregex = re.compile(r'''
([a-z0-9\.]+)
(@)
([a-z0-9\.]+)
(\.)
([a-z]{2,4})
''', re.VERBOSE | re.IGNORECASE)


phoneregex = re.compile(r'''
(\+\d)?
(-)?
(\d{3}|\(\d{3}\))?
(\.|-|\s)?
(\d{3})
(\.|-|\s)
(\d{4})
''', re.VERBOSE)

with open(filename, 'r') as filename:
    filedata = filename.read()
    emailmo = emailregex.findall(filedata)
    phonemo = phoneregex.findall(filedata)

print("Emails: \n")
for email in emailmo:
    for group in email:
        print(group, end='')
    print()

print()

print("Phone numbers: \n")
for pn in phonemo:
    for group in pn:
        print(group, end='')
    print()
\$\endgroup\$
1
\$\begingroup\$

The regular expression for email that matches 100% accurately would be a sight to see. One answer to this related question claims to do that. It certainly is horrendous, so I won't complain that yours isn't completely accurate. While a phone-number is relatively simple, the possibilities are more than you might think. For example, not all countries require ten digits. The ITU has defined the maximum number of digits to be 15. You can then take into account that the country code might not have a plus sign (+) before it. It might be something like 1-800-123-4567. The number of possibilities grows the more one thinks about it, so I won't complain. I'm merely pointing it out.


I would use re.finditer() instead of re.findall(). That means that instead of getting a list of tuples, you get an iterator of matches. With that, it becomes:

for email in emailmo:
    print(email.group())

and similar for the phone numbers.


There are two ways that I see to do the list comprehension (with re.finditer()):

  1. [print(pn.group()) for pn in phonemo]

  2. print('\n'.join([pn.group() for pn in phonemo]))

The first is positively sickening. It creates a list as long as the number of phone numbers or email addresses, and that list isn't even used. The second is a little more to my liking, but I do believe that the explicit for loop is better. For your curiosity, the old code in similar comprehensions could be horrendous:

[print(''.join(pn)) for pn in phonemo]

or ... horrendous:

print('\n'.join([''.join(pn) for pn in phonemo]))
\$\endgroup\$
0
\$\begingroup\$

All you really want to do to validate an email address is that you have at least one non-@ character, followed by an @ character, followed by at least one non-@ character.

Beyond that you're delving deep into the weird world of email rules - rules that are inconsistently followed and implemented. Trying to write a perfect regex is impossible (as I understand it, given that you can nest HTML comments inside of the email address, which everyone should agree is horrific, prevents it from being a regular language). Thus the two most important criteria are

  1. Can the service I use to send emails send to this address without choking?
  2. Will whoever I send this to click the "Confirm email address" link in the email?

You'll notice that answers to these questions give you different pieces of information about the validity of the email:

  1. Is the email address parseable (by my library)?
  2. Is the email address one that belongs to a real person?

Both are important questions to answer, and the best solution to both is to just send an email and see what happens. Until you send an email to a given address, and someone clicks a "confirm email address" link in that email, you don't know if that email is both valid (enough for your purposes) and real.

As a side note, if your library for sending emails has its own validation methods then feel free to use that - anything that their validation doesn't like is probably something they won't be able to send an email to. Otherwise do the bare bones check above and catch an error if it chokes.

\$\endgroup\$
  • \$\begingroup\$ Very detailed comment, but I forgot to mention that this is not a real project, this was just for practice with regex. \$\endgroup\$ – Aaron Aug 7 '16 at 1:14

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.