patternpythonMinor
Email checker ('@' sign, provider, domain extension)
Viewed 0 times
provideremailextensionsigndomainchecker
Problem
To learn about classes, I chose to create a little something that checks if you have included the following:
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,
- @ 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
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
Also, you could use better names in a few places. For example,
could be greatly improved if
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
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:
and then each instance will use the same lists. Speaking of, ...
Unnecessary constraints
Right now your
Concatenating strings
Instead of adding strings as
Readability
This isn't readable as is
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
I have some more critiques but I'm out of time - if I get a chance I'll post more later.
@ 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 expressionif 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.