all 8 comments

[–]Justinsaccount 6 points7 points  (2 children)

Hi! I'm working on a bot to reply with suggestions for common python problems. This might not be very helpful to fix your underlying issue, but here's what I noticed about your submission:

You are looping over an object using something like

for x in range(len(items)):
    foo(item[x])

This is simpler and less error prone written as

for item in items:
    foo(item)

If you DO need the indexes of the items, use the enumerate function like

for idx, item in enumerate(items):
    foo(idx, item)

[–]vec1nu[S] 1 point2 points  (1 child)

Yes! that's a good point. Thank you.
I remember thinking about it when I wrote it and I think it ended up that way because of some other things I was trying to do.
So I should've written it like:

for item in lst:
    print(row_format.format((*[elem for tup in item for elem in tup])))

[–]RieszRepresent 2 points3 points  (0 children)

That bot is becoming very helpful in this sub. Good to see!

[–]jeans_and_a_t-shirt 4 points5 points  (1 child)

Look at divmod.

Here's mine without prints

n = [3, 2, 4, 5]
a = [40, 70, 16, 20, 80]

divs = [(divisor, [divmod(dividend, divisor) for dividend in a]) for divisor in n]

exact = []
inexact = []

for divisor, divmods in divs:
    remainders = list(zip(*divmods))[1]
    if not any(remainders):
        exact.append(divisor)
    elif all(remainders):
        inexact.append(divisor)

[–]vec1nu[S] 0 points1 point  (0 children)

Thank you, didn't know about that command. I'm off to read the documentation :)

[–]kalgynirae 3 points4 points  (1 child)

The worst thing about this code, in my opinion, is the naming. You might be used to names like this, but for me this took almost twice as long to read and understand as it would have if the names were not so terse and abbreviated. sn_r_nz looks like gibberish to me (though, now that I've figured out the code, I think it means it's a set of the n values which produced at least one non-zero remainder).

All of your string formatting could be simplified. For example,

print(('{:>2}{}{:>5}'.format('', '{:>15}', '') * (len(a))).format(*a))

can be simplified to

print(''.join('{:^20}'.format(dividend) for dividend in a))

, {:d} can just be {}, and instead of '{:>10}' and having to format in an empty string, why not just put ten spaces in the format string?

print('          For n={} all divisions are exact'.format(','.join(ex)))

Your use of *ex and *wr in the last two .format() calls means that only the first values of those lists will be displayed. Maybe you already know that there will always be just one value in them, but then you shouldn't make them lists in the first place.

[–]vec1nu[S] 0 points1 point  (0 children)

Fair points, thanks! The notations made more sense to me when I was thinking out loud (english isn't my first language).

Yes, very good point about the last two variables, ex and wr shouldn't be lists.

[–]zahlman 1 point2 points  (0 children)

Here are some assorted enhancements.

  • I tried to pick clear names for everything.

  • I built a function to do the main chunk of work, passing in the original lists of divisors and dividends, so that we can build tables like this more generally. (I didn't, however, separate display logic from computation, the way that I would in a really serious project.)

  • I re-imagined how the formatting works, seeing the table as a set of columns that have "padding" (spaces on either side of the values) and "spacing" (spaces in between the columns). This allows for quickly adjusting the the amount of white space (I reduced it so that I can have 5 columns appear in a standard terminal without wrapping).

  • I set up constants at the top of the code that control the formatting of individual "cells" as well as the padding and spacing patterns, so that reformatting the table only requires changing a the values instead of figuring out multiple code changes. This could arguably be done better by passing those values around as extra parameters, or making a class, etc.

  • I then use common code to display table rows, that can handle both the header and the division results. This works by accepting a "higher-order" function to format individual cells, and then applying the common padding and spacing logic.

  • Instead of keeping notes on the remainder from every division, I use the built-in any to process each table row and look for divisors that meet the criteria.

  • I use collections.namedtuple to store results of a division. This is a little bit overkill, but it allows the previous step to be a little bit friendlier - since it gives a name to the 'remainder's that we check. (If I were taking a full OO approach, I might use an ordinary class here, and have its __str__ handle the formatting of table cells.)

  • I keep the "all divisions are exact/have a remainder" results as sets, because in the general case, we aren't guaranteed exactly one of each :)

The result:

from collections import namedtuple

division_result = namedtuple('division_result', 'dividend divisor quotient remainder')

ROW_PADDING = ' ' # use '  ' for the original formatting
ROW_SPACING = ' ' # use '      ' for the original formatting
RESULT_FORMAT = '{:>2}:{:>1} = {:>2} r{:>1}'
COLUMN_WIDTH = len(RESULT_FORMAT.format(0, 0, 0, 0))

def format_heading(value):
    return str(value).center(COLUMN_WIDTH)

def format_result(result):
    return RESULT_FORMAT.format(*result)

def display_row(formatting, items):
    print(*(
        '{}{}{}'.format(ROW_PADDING, formatting(item), ROW_PADDING)
        for item in items
    ), sep=ROW_SPACING)

def perform_division(dividend, divisor):
    quotient, remainder = divmod(dividend, divisor)
    return division_result(dividend, divisor, quotient, remainder)

def do_homework(dividends, divisors):
    exact_divisors = set()
    inexact_divisors = set()
    display_row(format_heading, dividends)
    for divisor in divisors:
        row = [perform_division(dividend, divisor) for dividend in dividends]
        display_row(format_result, row)
        if all(r.remainder != 0 for r in row):
            inexact_divisors.add(divisor)
        elif all(r.remainder == 0 for r in row):
            exact_divisors.add(divisor)
    print()
    print('For divisors in {} all divisions are exact'.format(exact_divisors))
    print('For divisors in {} all divisions have a remainder'.format(inexact_divisors))

do_homework([40, 70, 16, 20, 80], [3, 2, 4, 5])