4
\$\begingroup\$

I scripted a program in python to generate lottery numbers for user. There are two playing methods:

  • Lotto6aus49: User can choose any 6 numbers out of 49.
  • Eurojackpot: User can choose 5 numbers out of 50 and 2 numbers out of 12.

The script will generate some random numbers with above condition and generate a text file. Since I am new to python, I am not pretty sure, if my source code is efficient and python friendly. Are there any improvements I can do to my source code? Or are there any possibilities, where the source code can be minimized? Sourcecode:

import random
from datetime import datetime


date = datetime.now()
date_format = str(date.strftime('%d-%m-%Y %H:%M:%S'))
main_numbers = range(1, 50 + 1)
super_numbers = range(1, 12 + 1)
main_numbers_lotto = range(1, 49 + 1)
file_name = f'Lottozahlen generiert am '
lotto_name = f'Lotto 6aus49: '
euro_name_1 = f'Eurojackpot Zahlen: '
euro_name_2 = f'Eurojackpot Eurozahlen: '
coder = 'by mr.enso'
generated_time = f'generiert am: ' + date_format + ' '
coder_date_time = generated_time + coder
success_message = 'Die Text Datei wurde erfolgreich erstellt.'


def euro_number_generator():
    random_main_numbers = str(sorted(random.sample(main_numbers, 5)))
    random_super_numbers = str(sorted(random.sample(super_numbers, 2)))
    return random_main_numbers, random_super_numbers


# Assign individual variables to function euro_number_generator-output
main_numbers_output, super_numbers_output = euro_number_generator()


def lotto_number_generator():
    random_main_numbers_lotto = str(sorted(random.sample(main_numbers, 6)))
    return random_main_numbers_lotto


# Formatting output-numbers
final_lotto = lotto_name + lotto_number_generator().replace('[', '').replace(']', '')
final_euro_numbers1 = euro_name_1 + main_numbers_output.replace('[', '').replace(']', '')
final_euro_numbers2 = euro_name_2 + super_numbers_output.replace('[', '').replace(']', '')


def generate_textfile():
    f = open(file_name + date_format + f'.txt', f'w+')
    f.write(final_lotto + f'\n\n' + final_euro_numbers1 + f'\n' + final_euro_numbers2 + f'\n\n' + coder_date_time)
    f.close()


def print_to_terminal():
    print(f'\n' + final_lotto + f'\n' + final_euro_numbers1 + f'\n' + final_euro_numbers2 + f'\n\n' + coder_date_time)


def print_success_message():
    print(f'\n' + success_message + f'\n')


print_to_terminal()
generate_textfile()
print_success_message()

\$\endgroup\$

2 Answers 2

2
\$\begingroup\$

Layout

You are mixing functions and variables in your code, in general the structure of a file might look like:

  • Imports
  • Constants
  • Functions
  • Main Guard

Also, according to PEP-8, your constants should be in UPPER_SNAKE case.

It is also common practice to make function names verbs, i.e.

euro_number_generator
lotto_number_generator

become

generate_euros_numbers
generate_lotto_numbers

(Also, "generator" has a very specific meaning in Python which you will learn about later)

Compartmentalise

You declare a lot of variables at top-level which are only used within your functions. These should be kept local to where they're used:

main_numbers = range(1, 50 + 1)

Could be declared inside euro_number_generator, likewise for the other ranges (though main_numbers_lotto is unused), success_message (honestly should be just inside the print function inside a main guard, as it's always the same), etc.

Rather than having the functions generate_textfile and print_to_terminal we could have a function which returns the string used by both of these allowing them to simply be printed or written as needed.

Using Python Functions

Rather than constructing your strings by casting to string and replacing the brackets, it would be better to use Python's str.join() functions.

Instead of (effectively)

final_lotto = lotto_name + str(sorted(random.sample(main_numbers, 6))).replace('[', '').replace(']', '')

we can do:

lotto_nums = sorted(random.sample(main_numbers, 6))
final_lotto = f"{lotto name}: {", ".join(map(str, lotto_nums))}"  # Join takes lists of strings, so convert each to string with map

Also date.strftime returns a string, so we don't need to cast it.

F-strings

You are making your strings f-strings, but not using them as they are intended. F-strings substitute in-place for variables so that you don't have to concatenate strings together or use disjointed formats.

Thus:

generated_time = f'generiert am: ' + date_format + ' '

becomes

generated_time = f'generiert am: {date_format} '

or

f.write(final_lotto + f'\n\n' + final_euro_numbers1 + f'\n' + final_euro_numbers2 + f'\n\n' + coder_date_time)

becomes

f.write(f"{final_lotto}\n\n{final_euro_numbers1}\n{final_euro_numbers2}\n\n{coder_date_time}")

Although personally, given these are on different lines, I would split it into multiple writes (despite this being less efficient, efficiency here is not really a concern)

You also don't need to declare f-strings where you are not making such a substitution.

Arguments

You are not passing any arguments into your functions, which makes them a lot less flexible. At this point, they may as well be in-line statements. For example, in generate_textfile you take variables from the higher level to generate your filename, a better way might be:

def generate_textfile(filename, numbers):
    with open(filename, 'w') as out_file:  # Use `with` to auto close your files in case of e.g. errors
        out_file.write('\n'.join(numbers))
        out_file.write(f'\n\n{coder_date_time}')

generate_textfile(f'{file_name}{date_format}.txt', (final_lotto, final_euro_numbers1, final_euro_numbers2))

Purpose

As well as adding the main guard, one of the things we might want to do is make it so that the program generates only one as specified by the user.

Inside our main guard, we could, for example, check against any arguments passed in:

if __name__ == '__main__':
    import sys
    if not sys.argv:
        # generate_both
    else:
        for arg in sys.argv:
            arg = arg.lower()
            if arg == 'lotto':
                # Generate lotto
            elif arg == 'euro':
                # Generate euro
            else:
                raise ValueError(f'Unrecognised option {arg}, must be "lotto" or "euro".')
     print('Die Text Datei wurde erfolgreich erstellt.')

So that if the program is run as:

python my_lotto.py euro

The code will just generate euro lotto, if run as:

python my_lotto.py

Will generate both, etc.

Other arguments might be used to specify the filename to write to or other features, if they start getting complex, however, it may be useful to use tools such as argparse or click.

Summary

Just to give a rough idea of what it might look like keeping some of your structure and sketching out a few of the ideas:

"""
Generate lotto or euro lottery numbers and write to a file.

Call as:
$ python lotto.py
Generate both sets of numbers and print to screen

$ python lotto.py my_file
Generate both sets of numbers and write to 'my_file'

$ python lotto.py euro
Just generate euro numbers and print to screen

$ python lotto.py my_file lotto
Just generate lotto numbers and write to 'my_file'

$ python lotto.py lotto euro
Equivalent to no args
"""

import random
from datetime import datetime


def generate_euro_numbers():
    """ Generate numbers fitting the euro lottery draw """
    main_numbers = range(1, 50 + 1)
    super_numbers = range(1, 12 + 1)

    random_main_numbers = sorted(random.sample(main_numbers, 5))
    random_super_numbers = sorted(random.sample(super_numbers, 2))
    return random_main_numbers, random_super_numbers


def generate_lotto_numbers():
    """ Generate numbers fitting the 6aus49 lottery draw """
    main_numbers = range(1, 49 + 1)

    random_main_numbers_lotto = sorted(random.sample(main_numbers, 6))
    return random_main_numbers_lotto

def generate_lotto_string(lotto=None, euro=None):
    """ Constructs standard string form for lotto and euro numbers
    lotto: List of lotto numbers
    euro: Tuple of two lists of euro and super numbers
    """
    if not lotto and not euro:
        return ''

    lotto_name = 'Lotto 6aus49'
    euro_name_1 = 'Eurojackpot Zahlen'
    euro_name_2 = 'Eurojackpot Eurozahlen'

    date = datetime.now()
    date_format = date.strftime('%d-%m-%Y %H:%M:%S')
    coder = 'mr.enso'

    def to_str_list(arg):
        """
        Convert list to comma separated string
        """
        return ", ".join(map(str, arg))

    out_string = ''
    if lotto:
        out_string += f'{lotto_name}: {to_str_list(lotto)}\n'

    if euro:
        out_string += f'{euro_name_1}: {to_str_list(euro[0])}\n'
        out_string += f'{euro_name_2}: {to_str_list(euro[1])}\n'

    out_string += f'\nGeneriert am: {date_format} by {coder}'
    return out_string


def output_numbers(filename=None, lotto=None, euro=None):
    """
    Output lotto and euro numbers either to screen (if no filename) or to filename if provided
    """
    if not filename:
        print(generate_lotto_string(lotto, euro))
    else:
        with open(filename, 'w+') as out_file:
            out_file.writelines(generate_lotto_string(lotto, euro))


if __name__ == '__main__':
    import sys

    LOTTO, EURO = None, None
    # Default to printing to screen
    OUT_FILE = None

    # Naively assume arg 1 is filename (arg[0] is the filename)
    if len(sys.argv) > 1 and sys.argv[1] not in ('lotto', 'euro'):
        OUT_FILE = sys.argv.pop(1)  # Set OUT_FILE to user filename and remove from argument list

    if len(sys.argv) == 1:  # Called without other arguments
        LOTTO = generate_lotto_numbers()
        EURO = generate_euro_numbers()
    else:
        for arg in sys.argv[1:]:
            arg = arg.lower()
            if arg == 'lotto':
                LOTTO = generate_lotto_numbers()
            elif arg == 'euro':
                EURO = generate_euro_numbers()
            else:
                raise ValueError(f'Unrecognised option {arg}, must be "lotto" or "euro".')

    output_numbers(OUT_FILE, lotto=LOTTO, euro=EURO)
    print('\nDie Text Datei wurde erfolgreich erstellt.')
\$\endgroup\$
1
  • \$\begingroup\$ Thanks a lot. This will help me to understand python better. \$\endgroup\$ Sep 8, 2022 at 21:09
1
\$\begingroup\$

In one sense, your program is over-parametrised: the literal strings don't hold a lot of value in being assigned to individual variables. You're better off combining them into one Template string.

Recognize that you're generating three lotto sequences, that should all go through one function accepting two parameters: the figure count, and the highest figure possible.

Your '%d-%m-%Y %H:%M:%S' is close, though not exactly, equal to the default datetime format for the locale de-DE - this looks like Do 08 Sep 2022 17:52:04. If you can accept the minor changes (and I don't see why not), you would be better off assigning the locale properly and then using %c.

It's very important that filenames use a machine-sortable datetime format, usually ISO 8601.

Don't convert a list to a string and then apply replace(); instead .join on a separator of your choosing.

I do not believe that w+ is called for in this application; just use w.

Suggested

import random
from datetime import datetime
from locale import setlocale, LC_ALL
from string import Template
from typing import Iterable


BODY_TEMPLATE = Template(
'''Lotto 6aus49: $lotto

Eurojackpot Zahlen: $euro_1
Eurojackpot Eurozahlen: $euro_2

generiert am: $now von Herrn Enzo
'''
)


def generate_lotto(
    highest: int, n: int,
) -> list[int]:
    numbers = range(1, 1+highest)
    return sorted(random.sample(numbers, n))


def generate_body(
    lotto: Iterable[int],
    euro_1: Iterable[int],
    euro_2: Iterable[int],
    now: datetime,
) -> str:
    return BODY_TEMPLATE.substitute(
        lotto=', '.join(str(i) for i in lotto),
        euro_1=', '.join(str(i) for i in euro_1),
        euro_2=', '.join(str(i) for i in euro_2),
        now=now.strftime('%c'),
    )


def generate_textfile(
    body: str,
    now: datetime,
    stem: str = 'Lottozahlen generiert am ',
) -> None:
    with open(f'{stem}{now.isoformat()}.txt', 'w') as f:
        f.write(body)


def main() -> None:
    setlocale(LC_ALL, 'de_DE.UTF-8')

    now = datetime.now()

    body = generate_body(
        lotto=generate_lotto(50, 6),
        euro_1=generate_lotto(50, 5),
        euro_2=generate_lotto(12, 2),
        now=now,
    )
    print(body)
    generate_textfile(body, now)

    print('Die Text Datei wurde erfolgreich erstellt.')


if __name__ == '__main__':
    main()

Output

Lotto 6aus49: 8, 11, 13, 16, 40, 49

Eurojackpot Zahlen: 10, 12, 20, 41, 48
Eurojackpot Eurozahlen: 10, 11

generiert am: Do 08 Sep 2022 18:46:52  von Herrn Enzo

Die Text Datei wurde erfolgreich erstellt.
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.