I did the following Excercise from Automate the boring stuff with Python Chapter 7:

Write a function that uses regular exppressions to make sure the password string it is passed is strong. A strong password is defined as one that is at least eight characters long, contains both uppercase and lowercase characters, and has a least one digit. You may need to test the string against multiple regex patterns to validate its strengh.

My Solution:

I wrote four functions which check the individual aspects of the required password detection. Then these four functions are used to write the strong-password function which validates strings against all the requirements.

To test this i also checked out the unittest module.

Please let me know if theres sth to do better.

Is this a good way to test? Are these good test cases? Are there any bad practices/ little issues in the coding / testing?

Heres the code:

import re

def valid_length(string):
"""checks if length is > 8 to be a strong password"""
lenght_regex = re.compile(r'.{8,}')
if not lenght_regex.search(string):
return False
return True

def has_upper(string):
"""Check if string contains one upper letter or more"""
upper_regex = re.compile(r'.*[A-Z]+.*')
if not upper_regex.search(string):
return False
return True

def has_lower(string):
"""Check if string contains one lower letter or more"""
lower_regex = re.compile(r'.*[a-z]+.*')
if not lower_regex.search(string):
return False
return True

def has_digit(string):
"""Check if one or more signs is a digit"""
digit_regex = re.compile(r'.*\d+.*')
if not digit_regex.search(string):
return False
return True

"""
Validate if passed password is considered "strong",
- is eight characters or longer
- contains uppercase and lowercase characters
- has one digit or more
"""
return False
return False
return False
return False
return True


import unittest

"""Test of strong password detection function."""

def test_valid_length(self):
"""Test that only a string length of > 8 is accecpted"""
self.assertEqual(p.valid_length('abcd'), False)
self.assertEqual(p.valid_length('abcdefg'), False)
self.assertEqual(p.valid_length('abcdefgh'), True)
self.assertEqual(p.valid_length('abcdefghi'), True)

def test_has_upper(self):
"""Test that only strings containing uppercase are accepted"""
self.assertEqual(p.has_upper('abcd'), False)
self.assertEqual(p.has_upper('aBcd'), True)
self.assertEqual(p.has_upper('aBCd'), True)
self.assertEqual(p.has_upper('Abcd'), True)
self.assertEqual(p.has_upper('abcD'), True)
self.assertEqual(p.has_upper('ABCD'), True)

def test_has_lower(self):
"""Test that only strings containing lowercase are accepted"""
self.assertEqual(p.has_lower('abcd'), True)
self.assertEqual(p.has_lower('aBcd'), True)
self.assertEqual(p.has_lower('aBCd'), True)
self.assertEqual(p.has_lower('Abcd'), True)
self.assertEqual(p.has_lower('abcD'), True)
self.assertEqual(p.has_lower('ABCD'), False)

def test_has_digit(self):
"""Test that only strings containing lowercase are accepted"""
self.assertEqual(p.has_digit('abcd'), False)
self.assertEqual(p.has_digit('a1cd'), True)
self.assertEqual(p.has_digit('a12d'), True)
self.assertEqual(p.has_digit('1bcd'), True)
self.assertEqual(p.has_digit('abc1'), True)
self.assertEqual(p.has_digit('1234'), True)

"""
Test strong password function. Passed strings have to pass
all tests in valid_length, uppper, lower and digit functions.
"""

# Test from single functions should all fail
# (not met all criteria)

# Combinations which met more than one cirteria

if __name__ == '__main__':
unittest.main()

• Just for the record, these are really bad criteria for strong passwords – Oscar Smith Nov 28 '18 at 10:50
• @OscarSmith Yes in my answer I've added the obligatory xkcd as a note ;) – Ludisposed Nov 28 '18 at 10:56
• well it is just an excercise not a real time application – Sandro4912 Nov 28 '18 at 11:12

Good job on the easily understandable code.

# Good

• Good functions, with clear names!
• Modular approach
• Unittests
• Docstrings

# Improvements

• Regex with lots of backtracking can produce some major performance loss

Consider that this re.search(r'.*[A-Z]+.*', string)

is equal to re.search(r'[A-Z]+', string)

or even re.search(r'[A-Z]', string) as Toby correctly suggested.

Since we only care if one character is in the given string.

• Return directly

if exdpression:
return True
return False


Return directly with return expression

• Your compile has no performance gain, because with every new string it will compile again. Instead you could compile only onc,e and store it as a constant.

• Use the all keyword to check if all expressions evaluates to truthy.

• Instead of assertEqual(expression, function)

Do the more direct assertFalse or assertTrue

# Revised code

import re
import unittest

re.compile(r'[A-Z]'),
re.compile(r'.{8,}'),
re.compile(r'[a-z]'),
re.compile(r'[0-9]'),
]

"""
Validate if passed password is considered "strong",
- is eight characters or longer
- contains uppercase and lowercase characters
- has one digit or more
"""

"""Test of strong password detection function."""
"""
Test strong password function. Passed strings have to pass
all tests in valid_length, uppper, lower and digit functions.
"""

# Test from single functions should all fail
# (not met all criteria)

# Combinations which met more than one cirteria

• I don't think the + in any of those regexes buys anything valuable - "contains at least one" means that you can be content after matching exactly one. – Toby Speight Dec 4 '18 at 17:12