Simple Password Strength Checker Review Help needed

[Steven D'Aprano]

On Wed, 27 Jan 2010 11:26:21 +0530, Mallikarjun wrote:

I don't know much about pygtk and Glade, but I can make a couple of 
comments:


> class PassChecker:
>     
>     def on_check_button_toggled(self, button):
>         if self.pass_entry.get_visibility():
>             self.pass_entry.set_visibility(False)
>         else:
>             self.pass_entry.set_visibility(True)


This can be written as:


    self.pass_entry.set_visibility(not self.pass_entry.get_visibility())


Better still, get rid of the "set_visiblity" and "get_visibility" 
methods. This isn't Java, setters and getters are rarely needed. Just 
make an attribute "visible" and toggle it:

    self.pass_entry.visible = not self.pass_entry.visible



     

>     def on_guidelines_button_clicked(self, button):
>         message = "Here are some guidelines to be followed while
>         choosing password.\n\
[...]



It is best to write such long strings as a triple-quoted string, to avoid 
needing all those backslashes. Instead of:

    message = "Here are some guidelines... \n\
    blah blah blah \n\
    blah blah"

it is better to write:

    message = """Here are some guidelines...
    blah blah blah
    blah blah"""




>     def on_pass_entry_changed(self, text_entry):
>         password = self.pass_entry.get_text() strength =
>         self.password_strength_checker(password)
>         
>         if strength >= 0.0 and strength <= 0.20:
>             strength_in_str = "Very Week"

Python allows you to write multiple comparisons like this:

    if 0.0 <= strength <= 0.20:
        strength_in_str = "Very Weak"
    elif 0.20 < strength <= 0.40:
        strength_in_str = "Weak"


Note also the correct spelling of weak. ("Week" is seven days.)


>     def password_strength_checker(self, password):
>         
>         if not password:
>             return 0.0
>         
>         # Check for having same Characters
>         has_same_characters = True
>         first_letter = password[0]
>         for i in range(1, password.__len__()):

You nearly never need to explicitly call double-underscore methods like 
__len__. The above is better written as:


    for i in range(1, len(password)):


>             if not first_letter == password[i]:
>                 has_same_characters = False
>             if not has_same_characters:
>                 break
>         if has_same_characters:
>             return 0.0

I'm not sure what you're trying to do here. By the names, I think you're 
looking for any repeated characters, but judging by the code, you're only 
checking to see if the *first* character is repeated.

If you want to check only the first character, this is best written as:

    # get the first character, and see it if is in the rest of the string
    first_repeated = password[0] in password[1:]

If you want to check for any duplicate:

    any_duplicate = len(set(password)) != len(password)

If that's a bit too mysterious for you, try this:

    for c in password:
        # check each character for duplicates
        if password.count(c) != 1:
            print "character %s is duplicated" % c

I'm sure you can adapt that to do what you want.


>         lower_alphabets_count = 0
>         upper_alphabets_count = 0
>         digits_count = 0
>         special_chars_count = 0
>         for i in range(0, password.__len__()):

Again, use len(password).

>             char = password[i]

Since you *only* use i as an index to grab the ith character, this is 
better written by iterating over the string itself:

    for char in password:
        ...


>             if char.isalpha():
>                 if char.isupper():
>                     upper_alphabets_count += 1
>                 else:
>                     lower_alphabets_count += 1
>             elif char.isdigit():
>                 digits_count += 1
>             else:
>                 special_chars_count += 1

    if char.isupper():
        upper_alphabets_count += 1
    elif char.isupper():
        lower_alphabets_count += 1
    elif char.isdigit():
        digits_count += 1
    else:
        special_chars_count += 1


>         chars_count = [upper_alphabets_count, lower_alphabets_count,
>         digits_count, special_chars_count]
>         
>         if password.__len__() < 8:

len(password)

>             if not chars_count.__contains__(0):
>                 strength = 0.5

if not 0 in chars_count:
    strength = 0.5





Hope this helps.



-- 
Steven