2
\$\begingroup\$

What are nonogram tables?

Nonograms, also known as Hanjie, Picross or Griddlers, are picture logic puzzles in which cells in a grid must be colored or left blank according to numbers at the side of the grid to reveal a hidden picture.

There are two kind of nonogram tables, ones with only one row and column, and kinds with multiple rows and columns. For simplicity's sake, I have assumed that nonogram table given has only one row and one column, and 5x5 only. That is why my table is only nonogram-like, a true nonogram table gives out two columns if a row is as if colored-blank-blank-blank-colored. But in my code if the nonogram table is like that, the column will be 2 instead of 1 1.

Anyways, the code, in which I have assumed that blank spaces are 0 and painted spaces are asterisk. Keep in mind that blank space can be any character.

def nono_descriptor(given_string):
    number_of_lines = given_string.count('\n') #number of line breaks
    length_of_string = len(given_string) #length of the given string

    if number_of_lines != 4 and length_of_string != 29:
        raise Exception("You need exactly four line breaks and 25 characters!") #throw exception if conditions don't match

    rows_split = [] #list that contains each line split
    columns_list = [] #list that contains columns
    rows_list = [] #list that contains rows

    list_of_lines = given_string.split('\n') #split the string by line break

    for each_line in list_of_lines:
        rows_split.append(list(each_line)) #populate rows list with split up characters
        columns_list.append(each_line.count('*')) #populate columns list with the number of asterisks in each column

    for i in range(5):
        temp_list = [row[i] for row in rows_split]
        rows_list.append(temp_list.count('*'))

    print (columns_list, rows_list)


nono_descriptor('0*0*0\n***00\n****0\n*0*0*\n****0')

Would love to hear your opinions.

Again, don't mistake this code as a nonogram table generator. It only generates a table similar to that of a nonogram.

\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

Size of the grid

    raise Exception("You need exactly four line breaks and 25 characters!") #throw exception if conditions don't match

Using a generic Exception is bad practice. What should the calling code look like if it want to handle such failure? Something like

with open('da_file.txt') as picture:
    pic = picture.read()
try:
    descr = nono_descriptor(pic)
except Exception:
    print('Not the right size')
else:
    print(descr)

probably. Meaning that if anything goes wrong, it would be treated as if the input was not as expected.

You should at least use a more specific kind of exception (such as ValueError) or roll your own:

class BadGridSizeError(Exception):
    pass

def nono_descriptor(...):
    ...
    raise BadGridSizeException("...")

But why limit at 5×5 grids in the first place, why not let the user provide a grid with the size they like? It is easy to check that the grid is square and keep the size for further operations instead of hardcoding the value 5:

def nono_descriptor(given_string):
    list_of_lines = given_string.splitlines()
    grid_size = len(list_of_lines)

    assert all(len(row) == grid_size for row in list_of_lines), "Square grid expected"

    rows_split = [] #list that contains each line split
    columns_list = [] #list that contains columns
    rows_list = [] #list that contains rows


    for each_line in list_of_lines:
        rows_split.append(list(each_line)) #populate rows list with split up characters
        columns_list.append(each_line.count('*')) #populate columns list with the number of asterisks in each column

    for i in range(grid_size):
        temp_list = [row[i] for row in rows_split]
        rows_list.append(temp_list.count('*'))

    print (columns_list, rows_list)

Using an assert here is also more friendly to the user that can deactivate such checks (using the -O or -OO switch on the command line) when they know that their input is good.

You don't even need to restrict the grid to being square, you can have m×n grids if you check that each row is the length of the first one:

def nono_descriptor(given_string):
    list_of_lines = given_string.splitlines()

    assert list_of_lines, "At least one row expected"

    grid_size = len(list_of_lines[0])

    assert all(len(row) == grid_size for row in list_of_lines), "Rectangular grid expected"

    rows_split = [] #list that contains each line split
    columns_list = [] #list that contains columns
    rows_list = [] #list that contains rows


    for each_line in list_of_lines:
        rows_split.append(list(each_line)) #populate rows list with split up characters
        columns_list.append(each_line.count('*')) #populate columns list with the number of asterisks in each column

    for i in range(grid_size):
        temp_list = [row[i] for row in rows_split]
        rows_list.append(temp_list.count('*'))

    print (columns_list, rows_list)

Counting by rows and columns

You can easily get the count of '*' using a list-comprehension rather than using append():

columns = [row.count('*') for row in list_of_lines]

You can argue that it's better to build row_split in the meantime while iterating over list_of_lines… But you don't need that list. The only thing you use it for is to be able to index the ith column in each row. But you can do that exactly the same way using plain strings because list(my_str)[i] is exactly the same as my_str[i]. Meaning you can write:

columns = [row.count('*') for row in list_of_lines]

rows = []
for i in range(grid_size):
    tmp = [row[i] for row in list_of_lines]
    rows.append(tmp.count('*'))

However there are two improvements possible. The first one being using a list-comprehension:

rows = [[row[i] for row in list_for_lines].count('*') for i in range(grid_size)]

The second one being to use zip instead of trying to manually do its exact job:

rows = [colum.count('*') for column in zip(*list_of_lines)]

One more advantage using zip is that it will stop producing columns when the shortest row has been exhausted. Meaning you don't even need to check that the grid is rectangular anymore:

def nono_descriptor(given_string):
    lines = given_string.splitlines()

    columns = [row.count('*') for row in lines]
    rows = [column.count('*') for column in zip(*lines)]

    print(columns, rows)

print vs return

Printing a result within a function that perform some computations is bad practice. Your function can't be reused in a different context. What if I wanted to iterate over some files and use your function to append the various counts at the end of said file. Well I can't, because you print the result, so I can't do anything with it:

def nono_descriptor(given_string):
    lines = given_string.splitlines()

    columns = [row.count('*') for row in lines]
    rows = [column.count('*') for column in zip(*lines)]

    return columns, rows

Your use case should now be

print(nono_descriptor('0*0*0\n***00\n****0\n*0*0*\n****0'))

which might seem more work here, but it lets you reuse you function way more easily.

\$\endgroup\$
2
  • \$\begingroup\$ Thank you. I have a question, how can we compress this syntax: for i in range(n): \n for j in range(k) inside a list comprehension? Meaning, a list comprehension with two or more variables? \$\endgroup\$ Jul 13, 2016 at 12:33
  • \$\begingroup\$ @CodesInTheValley [stuff_using(i,j) for i in range(n) for j in range(k)] But you most probably want to iterate over the content rather than over indices, something like [stuff_using(letter) for row in grid for letter in row] \$\endgroup\$ Jul 13, 2016 at 12:35

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.