-
Notifications
You must be signed in to change notification settings - Fork 72
Added lesson 12 solutions #677
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
| return False | ||
| else: | ||
| return True | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
W391 blank line at end of file
|
@adziu lesson12 ready for review |
adziu
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The coin toss has a minor error. The email regex has a dangerous error for production code, but it shows understanding of regular expressions. If you have time, you can fix those, but I can accept that.
| guess = '' | ||
|
|
||
| toss_int = random.randint(0, 1) # 0 is tails, 1 is heads | ||
| if toss_int == 0: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a short form for that toss = 'tails' if toss_int else 'heads'
| print('You got it!') | ||
| else: | ||
| print('Nope! Guess again!') | ||
| guess = input() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You don't check the validity of input here. To avoid repeating the code you could enclose the while in a function.
|
|
||
|
|
||
| def pass_checker(some_password): | ||
| if re.match(r'^(?=.*[A-Z])(?=.*[a-z])(?=.*[0-9]).{8,}$', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's an interesting solution! In all the other reviews multiple regular expressions were used. 👍
| regex_text = re.compile(reg) | ||
| logging.info("Regex = {}".format(regex_text)) | ||
| files_list = os.listdir(folder) | ||
| logging.info("Files: {}".format(files_list)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's good to use logger that knows where you are. That is logger = logging.getLogger(__name__) at the beginning of the file and then logger.info(blah).
|
|
||
|
|
||
| def email_validator(some_email): | ||
| email_regex = re.compile(r"[^@]+@[^@]+\.[^@]+") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's more complicated than that. Not all the symbols are allowed. In real life you should search Google for "regex library" and get a comprehensive checker or use some library checker (Django for example has a field for that). In less real life situations you should take care that you accept a reasonable subset with _ . etc., but certainly not too much.
You accept too much. The input you accepted could cause some trouble. For example didn't put $ at the end, so the potential attacker could put something in the content of the mail message you are sending. You also accept spaces/inequality characters, semicolons, all rather dangerous, somebody could destroy your CSV file. [\w\d_.-]+ [a-z0-9_.-]+ would be much better than [^@]+ here. In other cases the [^ construct is the best way to go and good that you know it.
No description provided.