Program that should add a list to a dictionary and then clear it, clears it first and appends the subsequent empty list to dictionary?

Question

class Matrix:

rowStorer = {}
generalPurposeList = []

def __init__(self,row,column):
    self.row = row
    self.column = column #To store away for a later method that displays statistics about matrice

    for i in range(row):
        for j in range(column):
            numb = int(input('Enter A{}{}: '.format(i+1,j+1))) #i+1 and j+1 so I do not end up with 0 as starting value
            self.generalPurposeList.append(numb)

        self.rowStorer.setdefault(i+1,self.generalPurposeList)
        self.generalPurposeList.clear()


def __str__(self):
    megaMatrix = ''
    for i in self.rowStorer:
        megaMatrix += str(self.rowStorer[i])+'\n'
    return megaMatrix

testRide = Matrix(2,3)
print(testRide.__str__())

I am a beginner to Python classes and am trying to make a Matrix class as practice. The user specifies the number of rows and columns when creating a class instance.

Assuming row as m and column as n, an entry for any row m is taken n times using the input() in the for-loop. This number is then added to a list which is meant to contain that specific row's elements. The current row number is added to a dictionary as a key with the value being the list containing that specific row's elements

I then clear this list after its added to the dictionary as a key, to clean it up and ready it for the next row's elements. However, when I run the program, it seems to CLEAR the list first and then add it to the dictionary?

Im confused, what did I do wrong? Shouldn't it clear AFTER its been added to the dict as per the code?

I don't recommend baking user interaction into the constructor. This really limits reusability of the class. Move it to the caller and let them collect the input in a different module and provide it as a list to the matrix class. _I then clear this list after its added to the dictionary as a key, to clean it up and ready it for the next row's elements_ this is a pretty confusing design. I'd expect you want to collect a row, append it to the list of rows (dict is an odd choice here) and then create a new list for the next row. Clearing the same row list over and over won't work. — ggorlen, Apr 03 '21 at 19:35
@ggorlen Sorry, Im not able to understand. What do you mean exactly by 'I don't recommend baking user interaction into the constructor. Move it to the caller and let them collect the input in a different module and provide it as a list to the matrix class'? — Hash, Apr 03 '21 at 19:43
What if you want to initialize the matrix from a list or file rather than user input? Under your current design, you'd have to throw out the matrix class (or at least the constructor) and rewrite it to work with list input. Seldom do applications receive input from an interactive prompt, so writing a matrix class to only work in this narrow use case is considered poor design. I added an answer to illustrate. Let me know if anything's confusing. — ggorlen, Apr 03 '21 at 19:51
Does this answer your question? [List of lists changes reflected across sublists unexpectedly](https://stackoverflow.com/questions/240178/list-of-lists-changes-reflected-across-sublists-unexpectedly) — Tomerikoo, Apr 03 '21 at 19:57
You can't (shouldn't...) use a list as a `generalPurposeList`. Lists are mutable so reusing the same list will have you end up with the exact same list used everywhere. So whenever you do `list.clear()`, all your lists are now empty... Replace the `clear()` line with a new assignment i.e. `self.generalPurposeList = []`. But again, there is no sense in having that list as a class attribute — Tomerikoo, Apr 03 '21 at 19:59
@Tomerikoo Why does clearing the list clear out all my lists, but reassigning it as an empty list not have the same effect? — Hash, Apr 03 '21 at 20:12
Because doing `l = []` creates a new, freshly created list in memory and binds the name `l` to it. On the other hand doing `l.clear()` simply clears the contents of the same list `l` was referring to. You are re-using the same list reference all-over. If you would do `... = []` or use `.copy()`, you will be passing new list instances around. See the linked question above for more details. The accepted answer demonstrated that with the ids. You can play around in a shell with lists and call `id()` to get the hang of it — Tomerikoo, Apr 03 '21 at 20:15

ggorlen · Answer 1 · 2021-07-06T22:03:22.357

For starters, I don't recommend baking user interaction into the constructor. This unnecessarily limits reusability of the class. Move it to the caller and let them collect the input in a different module and provide it as an iterable to the matrix class.

I then clear this list after its added to the dictionary as a key, to clean it up and ready it for the next row's elements

Clearing the same row list over and over won't work because the dict references one instance of your list generalPurposeList from every key as described in List of lists changes reflected across sublists unexpectedly. Clearing one reference clears them all. I'd expect you want to collect a row, append it to the list of rows and then create a new list for the next row.

There is no advantage to making generalPurposeList an instance variable and this sort of premature optimization to avoid allocations is almost never done; that list should be in the narrowest scope needed, namely, inside the row for loop. The bug/confusion seems to be a direct consequence of this design decision.

Using a dict to implement a matrix is an odd choice; I'd use a list which is the best data structure for sequences of buckets from 0-n.

I also recommend using the dunder method __repr__ rather than __str__ which is automatically invoked by print. Either way, calling the dunder method directly is an antipattern; use str(foo) rather than foo.__str__() in most cases.

I suggest something like:

class Matrix:
    def __init__(self, rows=[]):
        self.rows = rows
    
    def __repr__(self):
        return "\n".join([str(x) for x in self.rows])

if __name__ == "__main__":
    def collect_int(
        prompt, 
        err_msg="Invalid number entered. Please try again."
    ):
        while True:
            try:
                return int(input(prompt))
            except ValueError:
                print(err_msg)

    mat = []
    rows = collect_int("Enter the number of rows: ")
    cols = collect_int("Enter the number of cols: ")

    for i in range(rows):
        mat.append([])

        for j in range(cols):
            mat[-1].append(collect_int(f"Enter A[{i}][{j}]: "))

    print(Matrix(mat))

Sample run:

Enter the number of rows: 2
Enter the number of cols: 4
Enter A[0][0]: gergerg
Invalid number entered. Please try again.
Enter A[0][0]: 0
Enter A[0][1]: 1
Enter A[0][2]: 2
Enter A[0][3]: 3
Enter A[1][0]: 4
Enter A[1][1]: 5
Enter A[1][2]: 6
Enter A[1][3]: 7
[0, 1, 2, 3]
[4, 5, 6, 7]

If the matrix class seems slim, that's fine -- it's responsible for what it should be responsible for under your current design and nothing more. I assume you'd add more methods to it like transpose, eye, etc, so it'll be more meaningful than just a pretty printer for 2d lists as it is now.

BTW, I'm sure you're doing this for educational purposes, but NumPy already implements optimized matrices so keep in mind you'll likely never write a matrix class like this in a real app.

Lastly, note I implemented only rudimentary error checking. Your collect_int function should probably accept a function predicate to prevent rows and columns from being negative numbers.

score 0 · Answer 2 · answered Apr 03 '21 at 19:55

The problem is that you are using self.generalPurposeList so self.rowStorer is saving a reference to that object instead of the list you want. Later, you clear that object using self.generalPurposeList.clear(), which is why you get an empty result.

change the line

self.rowStorer.setdefault(i+1,self.generalPurposeList)

to

self.rowStorer.setdefault(i+1,self.generalPurposeList.copy())

This adds a new list containing the values in the generalPurposeList list, and clearing the old list will not affect this one.

Program that should add a list to a dictionary and then clear it, clears it first and appends the subsequent empty list to dictionary?

2 Answers2

Linked