Independent of the actual programming language you break a lot of rules and recommendations.
First - you shall not repeat yourself. The snippet
if letter_one_ascii == 91 or letter_one_ascii == 123:
letter_one_ascii = 65
is repeated with slightly changed names
if letter_three_ascii == 91 or letter_three_ascii == 123:
letter_three_ascii = 65
or slightly changed values
elif letter_three_ascii == 92 or letter_three_ascii == 124:
letter_three_ascii = 66
Most probably you did not type that but you did copy-paste and edited names and/or values. This is very error prone as you could easily forget to change a name or value. Such errors are very hard to find. What would you do if you had to extend this generator to handle 5 characters? or 6?
Typical solution: write a function that shifts by n letters and another one shifting by one letter
def shift_by_one(letter):
# some magic here
return new_letter
def shift_by_n(letter, n):
# some magic here using previously defined function 'shift_by_one'
return new_letter
you are not repeating yourself any more and extending comes easy. finally you should implement another function shifting a word according to your rules
def shift_word(word):
# some magic here using previously defined function 'shift_by_n'
return new_word
Second - you shall name your magic numbers, especially if they are appearing more than once. but even if appearing only once, what does 124 stand for? or 66? This is hard to maintain for you and even harder for a different person.
Instead of 65 you could have something like
first_letter_ascii = ord('A')
Third - boundary checking. you can do that after incrementing like you did, but you can also do it before. The latter is preferred if outside values do not exist or do not make sense.
Instead of
letter_one_ascii = ord(letter_one)
letter_one_ascii += 1
if letter_one_ascii == 91 or letter_one_ascii == 123:
letter_one_ascii = 65
else:
letter_one = chr(letter_one_ascii)
you could easily write
if letter_one == 'Z' or letter_one == 'z':
letter_one = 'A'
else:
letter_one_ascii = ord(letter_one)
letter_one_ascii += 1
letter_one = chr(letter_one_ascii)
That is much more readable and by the way this would make the body for shift_by_one.
Other options for boundary checking are modulo operations
arr = 'ABCD'
print(arr[4%len(arr)])
and deliberatly extending arrays outside
arr = 'ABCDA'
print(arr[4])
Fourth - you handle a lot of special cases as you process uppercase and lowercase. however in your final output you are interested in uppercase only. convert month to uppercase immediately and eliminate all 90+ numbers respectively letter 'z'.
month = myDate.strftime("%b").upper()
this also enables you to use a modulo boundary check and we can eliminate the loop in shift_by_n
def shift_by_n(letter, n):
index = ord(letter) - ord('A')
index = (index + n) % 26
letter = chr(index + ord('A'))
return letter
Finally some python stuff:
you do not have to convert a string into a list to access characters, string provides slicing and iterability.
word = myDate.strftime("%b").upper()[:3]
if you end up in a list of characers and want to have a string you can join all characters by
''.join(mylist)
another nice feature in python is list comprehension which we use in shift_word
letters = [shift_by_n(letter,n+1) for n, letter in enumerate(word)]
we end up in
import datetime
def shift_by_n(letter, n):
index = ord(letter) - ord('A')
index = (index + n) % 26
letter = chr(index + ord('A'))
return letter
def shift_word(word):
letters = [shift_by_n(letter,n+1) for n, letter in enumerate(word)]
return ''.join(letters)
myDate = datetime.datetime.now()
word = myDate.strftime("%b").upper()[:3]
print "Your Password is: " + shift_word(word)
