9
\$\begingroup\$

I was given an assignment to ensure that a list was symmetric.

I built it in a way that worked and it passed the tests. But I wanted to see if it could've been written more efficiently and still readable.

Original question

A list is symmetric if the first row is the same as the first column, the second row is the same as the second column and so on. Write a procedure, symmetric, which takes a list as input, and returns the boolean True if the list is symmetric and False if it is not.

My solution

def symmetric(master):
    master_count = len(master)

    if not master: # checks to see if the list is empty
        return True

    if master_count != len(master[0]): #check to make sure the list is a square
        return False
    i = 0
    while i < master_count:
        j = 0
        while j < master_count:
            print str(master[i][j]) +" "+str(master[j][i])
            if master[i][j] != master[j][i]:
                return False
            j = j + 1 # close the loop
        i = i + 1 # close the loop
    return True

Tests & expected responses

print symmetric([[1, 2, 3],
                 [2, 3, 4],
                 [3, 4, 1]])
#>>> True

print symmetric([["cat", "dog", "fish"],
                 ["dog", "dog", "fish"],
                 ["fish", "fish", "cat"]])
#>>> True

print symmetric([["cat", "dog", "fish"],
                 ["dog", "dog", "dog"],
                 ["fish","fish","cat"]])
#>>> False

print symmetric([[1, 2],
                 [2, 1]])
#>>> True

print symmetric([[1, 2, 3, 4],
                 [2, 3, 4, 5],
                 [3, 4, 5, 6]])
#>>> False

print symmetric([])
#>>> False
\$\endgroup\$
  • 1
    \$\begingroup\$ The last output should be True, right? \$\endgroup\$ – Ashwini Chaudhary Feb 10 '15 at 0:24
  • \$\begingroup\$ Your code test with the input [1] will return an error! don't know how to solve \$\endgroup\$ – user178323 Aug 28 '18 at 13:08
7
\$\begingroup\$

You could use range (or xrange for Python 2) to make the code shorter and clearer.

So instead of

i = 0
while i < master_count:
    j = 0
    while j < master_count:
        print str(master[i][j]) +" "+str(master[j][i])
        if master[i][j] != master[j][i]:
            return False
        j = j + 1 # close the loop
    i = i + 1 # close the loop

We have

for i in range(master_count):
    for j in range(master_count):
        if master[i][j] != master[j][i]:
            return False

Actually we're doing twice the amount of work we need to. If master[i][j] == master[j][i], we don't need to check the opposite:

for i in range(master_count):
    for j in range(i + 1, master_count):
        if master[i][j] != master[j][i]:
            return False

Alternatively, you could use all and a generator expression:

return all(master[i][j] == master[j][i]
           for i in range(master_count)
           for j in range(i + 1, master_count))

I would also reconsider the variable names, e.g. matrix instead of master, dim or n instead of master_count.

\$\endgroup\$
3
\$\begingroup\$
  • The variable name master doesn't make any sense, as your function accepts a sequence containing sequences you should name it something that makes more sense. For eg we can use seq or iterable here:

  • row_count will make more sense than master_count.

  • if not master should be checked first, even before checking the length.

  • if master_count != len(master[0]) can fail we any of the rows after the first row contain different number of items. I've used any() with a generator expression in my answers below.

  • You can totally use a for-loop here, avoid indexing as much as possible in Python. Normal for-loops are more-readable.

My first suggestion is going to use indexing only to get the column items. To get the column items for a particular row I've created a function get_column that accepts an iterable and an index as its arguments and lazily yield item on at that index from each row of that iterable.

The looping over rows in done using the enumerate() function, it returns both the index and the current row. We can use this index to get the column items by passing it to get_column. Later we izip()(returns an iterator) the current row and the column items and check if any() of the items don't match:

from itertools import izip


def get_column(seq, ind):
    for row in seq:
        yield row[ind]


def symmetric(seq):
    if not seq: # checks to see if the list is empty
        return True

    row_count = len(seq)

    #check to make sure the list is a square
    if any(len(row) != row_count for row in seq):
        return False

    for ind, row in enumerate(seq):
        column = get_column(seq, ind)
        if any(row_item != col_item  for row_item, col_item in izip(row, column)):
            return False
    return True

Another way that doesn't use indexing at all.

Here I've used izip with splat operator *(explained in zip()'s docs), it is the recommended way to transpose an iterable in Python. Note that * will unpack the whole iterable at once, so if it was an iterator then we are actually consuming it all at once. The transposed iterator is than passed to imap so that each of transposed items(izip returns tuples) can be converted to list which is then going to us simply compare each row with a column using != operator, i.e no any() based for-loop for comparison here. It's weird that Python doesn't support comparing tuple with a list, hence this conversion to list is required.

But if the array if huge then the conversion to list and then comparing using != is actually going to be slow because it will require at least one complete loop over the tuple to convert it to a list, in that case we can remove imap call and within the loop simply compare the items using izip and any()(added this is comments).:

from itertools import izip, imap


def symmetric(seq):

    if not seq: # checks to see if the list is empty
        return True

    row_count = len(seq)

    #check to make sure the list is a square
    if any(len(row) != row_count for row in seq):
        return False

    columns = imap(list, izip(*seq))

    for row, col in izip(seq, columns):
        if row != col:
            return False
    # or
    # columns = izip(*seq)
    # for row, col in izip(seq, columns):
    #     if any(row_item != col_item for row_item, col_item in izip(row, col)):
    #         return False

    return True
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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