Refactoring a loop in Python-CodePudding

I know this is not good coding and I'm looking to improve it. I want to get the first name by default if no name is supplied. My hack still enters the for loop and if it would be a big one it would be innefficient. But if I do all the attributions that I'm doing in the for loop's inside if, again, in the outside if, I would duplicate code. And I wouldn't use a function just to set attributes so I'm not sure how to proceed here.

    if not name:
        name = self.data['big_list'][0]['name']

    for i in range(len(self.data['big_list'])):
        if self.data['big_list'][i]['name'] == name:
            self.name = name
            self.address = self.data['big_list'][i]['address']
            ...
            return

CodePudding user response：

if think there is a general bad practice or misunderstanding of class: there is a class right? and you check if your instance has the same name of one of the names in your list, and set itself.

-> in your instance it seems you have large data, containing a list of people AND attributes for one person. That does not sound correct.

Let say the class is called Person.

you can create an init() method, which takes one row of your data['big_list']. instead of setting each attribute in a loop.
you might also want to create a equals() method which checks if a person is the same as someone else. (check duplicates)
consider taking your large_data out of that class.

Could you provide us with a little more context?

CodePudding user response：

Here are some comments (that might be insuficient because I do not really understand what you want to achieve with the program).

The purpose of the for loop seems to be to find an item of the list self.data['big_list'] that meets the condition self.data['big_list'][i]['name'] == name, get some data and then terminate. Each entry of self.data['big_list'] is a dict.

This is a good job for a technique called list comprehension, which is much faster than for-looping.

The expression

filtered = [item for item in self.data['big_list'][1:] if item['name'] == name]

results in a list of dicts that are not the first one and meet the name-condition. The used expression self.data['big_list'][1:] is all of self.data['big_list'] but the first one. This technique is called slicing of lists.

There may be more than one entry in filtered, or, none. I assume you are only interested in the first match, because your program does return when a match happens. Therefore, the second part of the program would be

if len(filtered) > 0:
    first_match = filtered[0]
    self.name = name
    self.address = first_match['address']
    ...

This way the structure of the program is more clear and other readers can better understand what the program does. Furthermore, it is much faster ;-).