Andrew
Andrew

Reputation: 734

Defining a class property within __init__ as opposed to within another class method -- python

EDIT

Note, it was brought to my attention that Instance attribute attribute_name defined outside __init__ is a possible duplicate, which I mostly agree with (I didn't come upon this because I didn't know to search for pylint). However, I would like to keep this question open because of the fact that I want to be able to reinitialize my class using the same method. The general consensus in the previous question was to return each parameter from the loadData script and then parse it into the self object. This is fine, however, I would still have to do that again within another method to be able to reinitialize my instance of class, which still seems like extra work for only a little bit more readability. Perhaps the issue is my example. In real life there are about 30 parameters that are read in by the loadData routine, which is why I am hesitant to have to parse them in two different locations.

If the general consensus here is that returning the parameters are the way to go then we can go ahead and close this question as a duplicate; however, in the mean time I would like to wait to see if anyone else has any ideas/a good explanation for why.


Original

This is something of a "best practices" question. I have been learning python recently (partially to learn something new and partially to move away from MATLAB). While working in python I created a class that was structured as follows:

class exampleClass:
    """
    This is an example class to demonstrate my question to stack exchange
    """

    def __init__( self, fileName ):
        exampleClass.loadData( self, fileName )

    def loadData( self, fileName ):
        """
        This function reads the data specified in the fileName into the 
        current instance of exampleClass.
        :param fileName: The file that the data is to be loaded from
        """

        with open(fileName,'r') as sumFile:
            self.name = sumFile.readLine().strip(' \n\r\t')

Now this makes sense to me. I have an init class that populated the current instance of the class by calling to a population function. I also have the population function which would allow me to reinitialize a given instance of this class if for some reason I need to (for instance if the class takes up a lot of memory and instead of creating separate instances of the class I just want to have one instance that I overwrite.

However, when I put this code into my IDE (pycharm) it throws a warning that an instance attribute was defined outside of __init__. Now obviously this doesn't affect the operation of the code, everything works fine, but I am wondering if there is any reason to pay attention to the warning in this case. I could do something where I initialize all the properties to some default value in the init method before calling the loadData method but this just seems like unnecessary work to me and like it would slow down the execution (albeit only a very small amount). I could also have essentially two copies of the loadData method, one in the __init__ method and one as an actual method but again this just seems like unnecessary extra work.

Overall my question is what would the best practice be in this situation be. Is there any reason that I should restructure the code in one of the ways I mentioned in the previous paragraph or is this just an instance of an IDE with too broad of a code-inspection warning. I can obviously see some instances where this warning is something to consider but using my current experience it doesn't look like a problem in this case.

Upvotes: 2

Views: 769

Answers (1)

Bryan Oakley
Bryan Oakley

Reputation: 386030

I think it's a best practice to define all of your attributes up front, even if you're going to redefine them later. When I read your code, I want to be able to see your data structures. If there's some attribute hidden in a method that only becomes defined under certain circumstances, it makes it harder to understand the code.

If it is inconvenient or impossible to give an attribute it's final value, I recommend at least initializing it to None. This signals to the reader that the object includes that attribute, even if it gets redefined later.

class exampleClass:
    """
    This is an example class to demonstrate my question to stack exchange
    """

    def __init__( self, fileName ):
        # Note: this will be modified when a file is loaded
        self.name = None

        exampleClass.loadData( self, fileName )

Another choice would be for loadData to return the value rather than setting it, so your init might look like:

def __init__(self, fileName):
    self.name = self.loadData(fileName)

I tend to think this second method is better, but either method is fine. The point is, make your classes and objects as easy to understand as possible.

Upvotes: 2

Related Questions