3
\$\begingroup\$

The project outline:

Create a program multiplicationTable.py that takes a number N from the command line and creates an N×N multiplication table in an Excel spreadsheet.

My solution:

# A program to create a NxN multiplication table in excel
# Usage: python multiplication_table_maker.py "number" "folder to save the table in"

import openpyxl, sys
from openpyxl.utils import get_column_letter, column_index_from_string

def main(num, save_path):
    workbook = openpyxl.Workbook()
    sheet = workbook.active
    for sequence, index in enumerate(range(2, num + 2), 1):
        top = sheet.cell(row=1, column=index).value = sequence
        side = sheet.cell(row=index, column=1).value = sequence
    bottom_right = sheet.cell(row=num + 1, column=num + 1).coordinate
    for rows in sheet["B2":f"{bottom_right}"]:
        for cells in rows:
            top = int(sheet[f"{get_column_letter(cells.column)}1"].value)
            side = int(sheet[f"A{cells.row}"].value)
            cells.value = top * side
    workbook.save(f"{save_path}\multiplication_table_maker.xlsx")

if __name__ == '__main__':
    num = int(sys.argv[1])   # Enter a number
    save_path = sys.argv[2]
    main(num, save_path)
\$\endgroup\$

2 Answers 2

5
\$\begingroup\$

enumerate(range(2, num + 2), 1) is confusing, consider naming the argument as in the docs: enumerate(range(2, num + 2), start=1)

The name sequence is misleading since it stands for a single number. Same for rows and cells: should be row and cell.

top = int(sheet[f"{get_column_letter(cells.column)}1"].value)
side = int(sheet[f"A{cells.row}"].value)

Here you retrieve values from the excel table. Those values belong to the range you defined a few lines earlier, so just get them from there, it will be faster and more clear.

\$\endgroup\$
2
\$\begingroup\$

enumerate

To me, your enumerate(...) is backwards. Consider for example, a times table where you want only multiples of 3:

     3  6  9
   +--------
 3 | 9 18 27
 6 |18 36 54
 9 |27 54 81

Your row and column indices would start at 2, and go up by 1. This is the "enumeration" part. Your values would start a 3, and go up by 3, which would be the range() part. Ie, range(3, 10, 3).

So instead of:

    for sequence, index in enumerate(range(2, num + 2), 1):

I would have:

    for cell_index, value in enumerate(range(1, num + 1), start=2):

This is much clearer: the table values are generated from 1 to num (inclusive), and the table row/column indices start at 2.

save_path

    workbook.save(f"{save_path}\multiplication_table_maker.xlsx")

Why have you hard-coded the output filename in your function? Instead, use:

    workbook.save(save_path)

and let the caller specify the full filename:

    main(10, f"{save_path}\10x10.xlsx")
    main(20, f"{save_path}\20x20.xlsx")
    ...

main

Speaking of which, main(...) is an utterly undescriptive name. How about create_multiplication_table_spreadsheet(...)? A bit verbose perhaps, but very clear.

argparse

Instead of parsing command-line arguments yourself, try using the argparse module.

\$\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.