8
\$\begingroup\$

I wrote a function for drawing a table for data that comes from file:

import data_manager
import os

table = data_manager.get_table_from_file('games.csv')

len_of_longest_name = 0
i = 0
while i < len(table):
    if len_of_longest_name < len(table[i][1]):
        len_of_longest_name = len(table[i][1])
    i += 1

i = 0
len_of_longest_studio = 0
while i < len(table):
    if len_of_longest_studio < len(table[i][2]):
        len_of_longest_studio = len(table[i][2])
    i += 1
print("/", 6 * "-", "|", (len_of_longest_name - 2) * "-", "|", (len_of_longest_studio - 2) * "-", "|", "---", "\\")

# constructing string

i = 0
while i < len(table):
    string = '|'
    string += table[i][0] + '|'
    if len(table[i][1]) < len_of_longest_name:
        spaces = (len_of_longest_name - len(table[i][1])) * ' '
        string += table[i][1] + spaces + '|'
    else:
        string += table[i][1] + '|'
    if len(table[i][2]) < len_of_longest_studio:
        spaces = (len_of_longest_studio - len(table[i][2])) * ' '
        string += table[i][2] + spaces + '|'
    else:
        string += table[i][2] + '|'
    string += table[i][3] + '|'
    if len(table[i][4]) < 2:
        string += table[i][4] + ' |'
    else:
        string += table[i][4] + '|'
    if i < len(table) - 1:
        print(string)
        print("|", 6 * "-", "|", (len_of_longest_name - 2) * "-", "|", (len_of_longest_studio - 2) * "-", "|", "---", "|")
    else:
        print(string)
    i += 1
print("\\", 6 * "-", "|", (len_of_longest_name - 2) * "-", "|", (len_of_longest_studio - 2) * "-", "|", "---", "/")

The code works as I want but I think there's a better (shorter) method to do the same. Could you show me the method?

The output looks like this:

Output

\$\endgroup\$
5
  • \$\begingroup\$ what was the source data like? \$\endgroup\$
    – hjpotter92
    Commented Jan 16, 2018 at 18:44
  • \$\begingroup\$ kH34Ju#&;Age of Empires II: The Age of Kings;Ensemble Studios;32;32\n Every record looks like this, ";" is a separator. \$\endgroup\$
    – Voyt
    Commented Jan 16, 2018 at 19:10
  • 2
    \$\begingroup\$ Have you considered using a package such as terminaltables? \$\endgroup\$ Commented Jan 16, 2018 at 20:09
  • \$\begingroup\$ what is data_manager? \$\endgroup\$ Commented Jan 17, 2018 at 4:17
  • \$\begingroup\$ data_manager is a module that reads from file and returns list of lines in one variable \$\endgroup\$
    – Voyt
    Commented Jan 17, 2018 at 7:56

3 Answers 3

3
\$\begingroup\$

Python has many great ways to iterate. In general if you find yourself looping on an integer, like in the following code, there is a good chance you are doing it wrong. It is generally better to iterate on the data structure itself.

Sample code:

I will use this loop as an example:

len_of_longest_name = 0
i = 0
while i < len(table):
    if len_of_longest_name < len(table[i][1]):
        len_of_longest_name = len(table[i][1])
    i += 1

Don't iterate on the length of a data structure:

This is probably not what you want:

while i < len(table):

It would likely be better as something like:

for row in table:

Or, if for some reason you really need the index, then:

for i, row in enumerate(table):

Example with looping on length removed:

So removing i, and looping on table, we get:

len_of_longest_name = 0
for row in table:
    if len_of_longest_name < len(row[1]):
        len_of_longest_name = len(row[1])

Use builtin functions.

Python have a max() function. Using max(), the example becomes:

len_of_longest_name = 0
for row in table:
    len_of_longest_name = max(len_of_longest_name, len(row[1]))

Comprehensions:

Since max() can take a comprehension the example loop can be reduced to:

len_of_longest_name = max(len(row[1]) for row in table)

That is six lines of code reduced to one.

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

Starting from what @Stephen Rauch said in his answer, you can greatly simplify your code.

  • You should definitely take a look at str.join, enumerate, zip and why you should not do string addition when concatenating many (not short) strings.

  • I would first compute all column lengths (not just the two you calculate, relying on the fact that the other lengths are the same across rows). For this you can use nested list comprehensions.

  • I would declare the repeatedly re-used strings which make the table frame only once, instead of building them every loop iteration.

  • Finally, I would put this into a function to make it re-usable and add a if __name__ == "__main__": guard to allow importing this function from another script without running the code.

def data_table(table)
    column_lens = [max(len(row[i]) for row in table)
                   for i in range(len(table[0]))]

    # constructing string
    separator = " | ".join("-" * (column_len - 2) for column_len in column_lens)
    header = "/ " + separator + " \\"
    row_separator = "| " + separator + " |"
    footer = "\\ " + separator + " /"

    new_table = [header]
    for i, row in enumerate(table):
        new_row = "|".join(column + " " * (column_len - len(column))
                           for column, column_len in zip(row, column_lens))
        new_table.append("|" + new_row + "|")
        if i < len(table) - 1:
            new_table.append(row_separator)
    new_table.append(footer)
    return "\n".join(new_table)

if __name__ == "__main__":
    table = data_manager.get_table_from_file('games.csv')
    print(data_table(table))

This prints for example:

/ ------ | --------------------------------- | -------------- |  |  \
|kH34Ju%&|Age of Empires II: The Age of Kings|Ensemble Studios|32|32|
| ------ | --------------------------------- | -------------- |  |  |
|kH34*&23|Age of Empires I                   |Ensemble Studios|32|32|
\ ------ | --------------------------------- | -------------- |  |  /

Note that this is a slightly different format (it has no sub-cells, as you do in your last two columns). Also note that if a column contains only cells of width 1 or 0, then the header and the table will be misaligned (since the header and footer row is at least 2 characters wide for each column.

Compared to your code, this has the great advantage that it works for every table that is defined as a list of lists.

\$\endgroup\$
-1
\$\begingroup\$

Thanks for your answers. I wrote this:

import data_manager

def table_lines():
    string = ""
    for indices in range(0, len(max_len)):
        value = max_len[indices]
        string += "|" + (value * "-")
    string += "|"
    return string


def data_lines(table):
    data_string = "|"
    for row in table:
        for indices in range(0, len(row)):
            if len(row[indices]) < max_len[indices]:
                data_string += row[indices] + ((max_len[indices] - len(row[indices] )) * " ") + "|"
            else:
                data_string += row[indices] + "|"
        print(table_lines())
        print(data_string)
        data_string = "|"



def get_longest(table):
    for row in table:
        value_len = []
        for element in row:
            value_len.append(len(element))
        # for debug print(row)
        values_len.append(value_len)
    # for debug print(values_len)

    for row in values_len:
        for indices in range(0, len(row)):
            try:
                if row[indices] > max_len[indices]:
                    max_len[indices] = row[indices]
            except IndexError:
                max_len.append(row[indices])

The output looks like this: enter image description here

Now I will work on fixing a corners of table. Thanks once more!

\$\endgroup\$
1
  • 1
    \$\begingroup\$ You should not be (implicitly even) using globals like you do. Instead return the objects you create inside your functions and pass them as arguments to the functions that need them. \$\endgroup\$
    – Graipher
    Commented Jan 17, 2018 at 15:53

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.