What you have posted is not complete since it is missing definitions and will not execute as is.
What is this function supposed to do?
The title of your post explains in slight detail what the function does but nowhere in the actual code is there a description. The name of the function, generate
, is not descriptive: What does this function "generate." Surely a more descriptive name might be generate_password
or generate_random_password
.
Looking at your code, the main loop is:
while _counter < OperationalVariables.input_desired_length:
_first_random_number = randint(0,len(_what_to_generate_from) - 1)
_second_random_number = randint(0, len(_what_to_generate_from) - 1)
if _first_random_number == _second_random_number:
_characters_list.append(_what_to_generate_from[_first_random_number])
_counter += 1
What are you trying to accomplish by generating two indices and throwing away the second one? What is this code supposed accomplish? You may or may not end up with two successive characters of the password being the same. So what is the value in generating the second random number? This code is in great need of comments here.
Improved variable naming
As I mentioned in my comment to your post variables that are local to a function are not visible to code outside of the function and therefore there is no need to suggest that they should be considered "private" by naming them with leading underscores.
You should also use more descriptive variable names.
The function argument, what_to_generate_from, should be a string
First, a more descriptive name for this argument might be legal_characters
.
Instead of having this argument be a list of single-character strings, it could more simply be just a single string. Otherwise, you should be checking that each element of the list really is a single-character string. The choice of making this argument a string also will result in more efficient code as we will see below.
Your code could be more efficient
You are generating a random integer and using that to index into your what_to_generate_from
(i.e. legal_characters) list. Note that you could just as easily use this to index into the what_to_generate_from
argument if it were a single multi-character string. Moreover, having this argument being a string allows us to more directly get the next password character by using the randint.choice
function (see below).
If you are required by the function specification that this argument must be a list of strings, then I would suggest that it be converted into a single string to achieve the aforementioned efficiency with:
what_to_generate_from = ''.join(what_to_generate_from)
By joining together the list elements into a single string also removes the limitation that each element be a single-character string.
You have an unnecessary counter
variable whose value shadows the length of your characters_list
list. I think it would be better for sake of conciseness and clarity to not use the counter
variable at all.
Having generated a list of single-length characters that you want to append together to form the generated password, you have chosen to loop through the list performing string concatenation operations. A far more efficient method would be to use the built-in join
method:
password = ''.join(characters_list)
Updated code with suggested changes
Since I cannot be sure what you were trying to accomplish by generating two random integers each time your loop, I have decided that in my updated version I would ensure that no two successive characters of the password are the same:
from random import choice
PASSWORD_LENGTH = 10
def generate_password(legal_characters : str):
"""Generate random characters based on the list given
Args:
legal_characters (str): The list of legal characters to be used
Raises:
ValueError : If the passed argument is not a str or its
length is less than 2
Returns:
str : The random character's generated
----- This Functions Is not for prior use , It is only be used in generate_random_password function.
"""
if not type(legal_characters) is str:
raise ValueError('Argument legal_characters must be a string')
# To ensure that we do not have duplicate characters adjacent in the password:
if len(legal_characters) < 2:
raise ValueError('Argument legal_characters must be at least 2 characters long')
password_chars = []
while len(password_chars) < PASSWORD_LENGTH:
password_char = choice(legal_characters)
# Ensure that this is not a duplicate of the previously
# generated character:
if len(password_chars) == 0 or password_char != password_chars[-1]:
password_chars.append(password_char)
return ''.join(password_chars)
if __name__ == '__main__':
print(generate_password('abcdef012345_'))
Prints:
03bdcfd1c5
any
isn't correct. You shouldfrom typing import Any
, as the lowercase version is actually the built-in functionany
. \$\endgroup\$