HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

Email checker ('@' sign, provider, domain extension)

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
provideremailextensionsigndomainchecker

Problem

To learn about classes, I chose to create a little something that checks if you have included the following:

  • @ sign



  • Provider (e.g. 'hotmail')



  • Domain extension (e.g. '.com')



  • It also checks if you entered spaces in the address and gave the option to keep them or get rid of them.



I would really like some feedback on what I should improve on. If you have any questions about why I did what I did at a specific part of the code, ask away!

```
class email_checker:

def __init__(self, email_input):
self.email_input = email_input
self.done = False
self.fixed_domain = False
self.redo = True
self.answer = ''
self.domain_extension = ''
self.providers = ['hotmail', 'gmail', 'yahoo', 'outlook']
self.de = ['com', 'org', 'net', 'edu', 'gov']

def enter_email(self):
self.email_input = input('Enter your email \n>')
print('You entered: ' + self.email_input)
self.provider_check()

def provider_check(self):
for i in self.providers:
if i in self.email_input:
self.at_check()
return # I use this to exit this method when it returns from
# self.de_check()
# Making sure that it doesn't run the
# self.enter_email() method again.
print('Please include which provider you\'re using. E.g. outlook')
self.enter_email()

def at_check(self):
if not '@' in self.email_input:
print('----\nYou missed the \'@\' sign! Let me fix that for you.')

for i in self.providers:
if i in self.email_input:
self.email_input = self.email_input.replace(i, '@' + i)
self.space_check()

def space_check(self):
if ' ' in self.email_input:
self.fixed_email = self.email_input.replace(' ', '')
print('----\nPlease choose:\n1: %s\n2: %s' % (self.fixed_email,

Solution

My first comment would probably be that you shouldn't try to validate emails beyond checking that they have at least one character before an @ sign, one . after the @ sign, and at least one character after the .. The reason for this is that validating emails is hard (those are each separate links, fwiw). In general,


the only valid email address is one you can send email to

No matter how hard you try to validate that the email is valid in terms of format, it will never check for nonsense email addresses ("asdf@asd.com") or typoes ("my_anme@gmail.com"). So practically speaking, email validation is generally not worth a whole lot unless it is obviously wrong ("asdf").

That being said, I'll critique your code, specifically.

Naming

According to PEP8 the de-facto* code quality standard for Python, class names should be in PascalCasing, so you should rename the class EmailChecker.

Also, you could use better names in a few places. For example,

for i in self.providers:
    if i in self.email_input:
        self.at_check()


could be greatly improved if i were replaced by something like provider_name.

Documentation

You should include docstrings for all public classes, methods, functions, and modules. This will help you use your code later, and help others as well.

Constants and class-level attributes

Right now you have these values

self.providers = ['hotmail', 'gmail', 'yahoo', 'outlook']
self.de = ['com', 'org', 'net', 'edu', 'gov']


as instance variables, and you set them individually for each instance of your checker. Instead, you could reduce repetition and make them class-level attributes:

class EmailChecker:
    providers = ['hotmail', 'gmail', 'yahoo', 'outlook']
    de = ['com', 'org', 'net', 'edu', 'gov']


and then each instance will use the same lists. Speaking of, ...

Unnecessary constraints

Right now your providers and de are far from comprehensive - what if my email isn't in any of those, but is still valid? You're better off validating that the provider and domain extension look reasonable. If you're interested, this link shows an (incomplete) list of top-level domain extensions.

Concatenating strings

Instead of adding strings as "string" + input("some string"), use string.join or string.format. They are more efficient, and much easier to read.

Readability

This isn't readable as is

if not 'com' in self.email_input and not 'org' in self.email_input \
and not 'net' in self.email_input and not 'edu' in self.email_input \
and not 'gov' in self.email_input:


at the very least indent the 2nd and 3rd lines. Additionally, it is generally preferred to use parentheses to have multi-line statements instead of the \ character. Even better, however, would be a generator expression

if not any(domain_extension in self.email_input for domain_extension in self.de):


I have some more critiques but I'm out of time - if I get a chance I'll post more later.

Code Snippets

for i in self.providers:
    if i in self.email_input:
        self.at_check()
self.providers = ['hotmail', 'gmail', 'yahoo', 'outlook']
self.de = ['com', 'org', 'net', 'edu', 'gov']
class EmailChecker:
    providers = ['hotmail', 'gmail', 'yahoo', 'outlook']
    de = ['com', 'org', 'net', 'edu', 'gov']
if not 'com' in self.email_input and not 'org' in self.email_input \
and not 'net' in self.email_input and not 'edu' in self.email_input \
and not 'gov' in self.email_input:
if not any(domain_extension in self.email_input for domain_extension in self.de):

Context

StackExchange Code Review Q#110593, answer score: 9

Revisions (0)

No revisions yet.