Assess the use of parent member variables by child classes

[MatthewHambley]

In a number of places we see this pattern:

class Parent:
    def __init__(self):
        self._member: Optional[str] = None

    @abstractmethod
    def method(self, thing: str) -> None:
        self._member = thing

class Child(Parent):
    def __init__(self):
        super().__init__()

    def method(self, thing: str) -> None:
        super().method(thing)
        do_something_to(self._member)

I am concerned that an ostensibly private member variable of the parent is being used in this manner. It seems messy and breaks encapsulation.

We need to consider whether there is a better, neater way to achieve the desired result.

This pattern is seen in: fab.steps.grab.GrabSourceBase, fab.steps.__init__.Step, fab.steps.compile_fortran.CompileFortran, fab.tasks.c.CAnalyse and fab.tasks.fortran.FortranAnalyse.

[bblay]

From an purely OO perspective, ignoring Python's idiosyncratic implementation, I'd argue that it's not a private member of the parent at all, it's a ~private~ protected member inherited by the child.

In Python lingo: there is no ~spoon~ parent instance, only a child instance. There is only one instance attribute and only one instance for it to belong to. It certainly doesn't belong to a class.

[bblay]

It's also worth pointing out that this pattern is used to provide attributes which are only set at runtime, for child processes to read. They can't modify memory in the parent process, they work on a copy. Didn't spend a lot of time looking for nicer pattern but keen to see if there are any. Perhaps they don't belong in the constructor at all.

[bblay]

The incremental psyclone work built up excessive "#210-isms", so they were bundled up into a data class and passed in. Reads and runs fine.

x90s = artefact_store['preprocessed_x90']
mp_payload = self.analysis_for_prebuilds(artefact_store)
mp_arg = [(x90, mp_payload) for x90 in x90s]
self.run_mp(mp_arg, self.do_one_file)