python instance variables initialization code smell?-CodePudding

In the two self.load_file() methods below, the first one has no arguments and uses the instance variables self.a and self.b, and the second one uses a and b directly as input arguments, and load_file essentially becomes a static method. They both achieve the same purpose, which one is better/preferred, and why?

Secondly, if I had additional instance variables that depend on the previous one, e.g. self.d = self.do_something_with_file() and self.e = self.do_something_with_d(), is it a code smell if there is some sort of a consecutive dependency in which the instance variables are created? For example, if there is an unhandled exception during self.file = self.load_file(), then the consecutive chain below (i.e. self.d and self.e) will also break. Is it common to have this 'consecutive dependency' (I am not sure if there is a term for this) style instantiation in practice (especially in larger/production scale programs)?

class SomeClass:

    def __init__(self, a, b):
        self.a = a
        self.b = b
        self.file = self.load_file()

    def load_file(self):
        for file in os.listdir(config.dir):
            file_split = file.split("_")
            file_a, file_b = file_split[0], file_split[2]

            if file_a == self.a and file_b == self.b:
                try:
                    return pickle.load(open(config.dir   '{}'.format(file), 'rb'))
                except OSError as e:
                    raise Exception("pickle.load error.") from e
        else:
            raise FileNotFoundError('file not found.')

class SomeClass:

    def __init__(self, a, b):
        self.a = a
        self.b = b
        self.file = self.load_file(a, b)

    @staticmethod
    def load_file(a, b):
        for file in os.listdir(config.dir):
            file_split = file.split("_")
            file_a, file_b = file_split[0], file_split[2]

            if file_a == a and file_b == b:
                try:
                    return pickle.load(open(config.dir   '{}'.format(file), 'rb'))
                except OSError as e:
                    raise Exception("pickle.load error.") from e
        else:
            raise FileNotFoundError('file not found.')

CodePudding user response：

as already written, this is matter of experience and personal taste, but if you, let say you have a UtilityClass, which encapsulates this specific function(s) you will be better off, if you need to change something in the the function, without to take care of where this function will be used:

class UtilityClass:

    @staticmethod
    def load_file(a,b):
       ...

class A:
   def __ini__(self, a, b):
       self.a = a
       self.b = b
       self.file = UtilityClass.load_file(a,b)

Good luck :)

CodePudding user response：

I would use a utility module with a simple function.

# utility.py
from pathlib import Path
import pickle

# default path
default_file = Path("object.foo")

def load_file(file_path: Path = default_file):
    with file_path.open("rb") as file:
       return pickle.load(file)


# main.py
from utility import load_file


class Foo:
    ...

try:
    f = load_file()
except FileNotFoundError:
    f = Foo()
    
assert isinstance(f, Foo)

so the file loader could be used for any object easily.