3
\$\begingroup\$

I was doing a programming challenge and the object was to validate an email address, I wrote a simple little email validator and would like some input on how I can make it more efficient.

How it works, it has two validations, one with a regex, and one validating the length (most email addresses are over 12 characters, so that's what I went with). The regex checks for an @ and a . symbol. This is all inside of its own validation class.

Source:

import re


class Validator(object):

    def __init__(self, email):
        self.email = email

    def check_for_symbol(self):
        if re.search("[@.]", self.email) is None:
            return False
        else:
            return True

    def check_length(self):
        if len(self.email) >= 12:
            return True
        else:
            return False


def email_to_verify():
    return raw_input('Enter your email address: ')


def check_email(email):
    validator = Validator(email)
    symbol = validator.check_for_symbol()
    if symbol is False:
        print "Your email address is not valid. Failed symbols.."

    length = validator.check_length()
    if length is False:
        print "Your email is not valid. Failed length"

    if length and symbol is True:
        print "Email is a valid email address."


if __name__ == '__main__':
    e = email_to_verify()
    check_email(e)

Is there a way to make this more efficient?

Examples of usage:

C:\challenges>python email_val.py
Enter your email address: test
Your email is not valid. Failed symbols check.
Your email is not valid. Failed length check.

C:\challenges>python email_val.py
Enter your email address: [email protected]
Your email is not valid. Failed length check.

C:\challenges>python email_val.py
Enter your email address: [email protected]
Email is a valid email address.
\$\endgroup\$
6
  • 5
    \$\begingroup\$ I'm really curious to know why you consider an address with less than 12 characters not an email address. It's not because most email address are > 12 characters that email address with < 12 characters are not valid. \$\endgroup\$
    – Marc-Andre
    Commented Aug 30, 2016 at 19:23
  • \$\begingroup\$ @Marc-Andre Honestly, just through 12 in there because 90% of the people this is made for have their first and last names as their email address. \$\endgroup\$ Commented Aug 30, 2016 at 19:38
  • 4
    \$\begingroup\$ The code claims that aaaaaa.bbbbbb@ is a valid address... \$\endgroup\$
    – vnp
    Commented Aug 30, 2016 at 19:40
  • \$\begingroup\$ @vnp That's outstanding lol \$\endgroup\$ Commented Aug 30, 2016 at 19:47
  • \$\begingroup\$ Note that context-dependent email addresses don't necessarily have a fully-qualified DNS name - in many organisations, partial names are resolved (e.g. within example.com, an address such as controller@finance is exactly equivalent to [email protected]. \$\endgroup\$ Commented Dec 18, 2017 at 15:37

1 Answer 1

9
\$\begingroup\$

Interface design

The current design of the validator class is not very intuitive. Users of this class must call the check_for_symbol and check_length methods to validate an email address. They would only know this by reading the implementation. You could add documentation, but it would still not be easy to use. The best is when the usage and behavior is obvious.

I would imagine the design of an email validator something like this:

class EmailValidator:
    def validate(self, email):
        # ...

Where the validate method would raise exceptions depending on what failed (such as length, symbols).

With such outline, even without documentation, when a user does help(EmailValidator) and sees a single validate method, the usage is obvious, clear.

What's worse, the check_email function is not part of the validator class. As such, the validator doesn't encapsulate the validation logic. That might be acceptable if the public API is the check_email and the class is the implementation detail.

Coding style

Use boolean expressions directly. For example, instead of this:

def check_for_symbol(self):
    if re.search("[@.]", self.email) is None:
        return False
    else:
        return True

You could write as:

def check_for_symbol(self):
    return re.search("[@.]", self.email) is not None

Also, instead of x is True, use simply x, and instead of x is False, use not x.

See more on this in the style guide.

Compatibility with Python 3

When using Python 2, to future-proof yourself it's good to adopt a writing style compatible with Python 3. For example, instead of print 'something', write print('something').

\$\endgroup\$
3
  • \$\begingroup\$ Would you consider the boolean expressions directly to be more readable? It seems that using that directly could be confusing. \$\endgroup\$ Commented Aug 30, 2016 at 19:50
  • 1
    \$\begingroup\$ Yes, naturally more readable. Note also that this is explicitly recommended by the style guide. \$\endgroup\$
    – janos
    Commented Aug 30, 2016 at 19:52
  • \$\begingroup\$ sorry for the easy question, I have no idea what I'm doing in case you couldn't tell lol. Thank you! \$\endgroup\$ Commented Aug 30, 2016 at 20:00

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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