CODEWITHSUNDEEP
pythonoopinheritance
Daniel Pop
Daniel Pop

Reputation: 551

python - remain in base class when calling parent method from child

I have the following base class:

class ClientRepo(Repository):

    def __init__(self) -> None:
        self.__clientList = []

    def hasClientWithId(self, clientId):
        for client in self.__clientList:
            if client.getId() == clientId:
                return True
        return False

    def addClient(self, client):
        if type(client).__name__ == 'ClientDAO':
            if not self.hasClientWithId(client.getId()):
                client.setClientId(self.__maximumIndexInClientList() + 1)
                self.__clientList.append(client)
            else:
                raise ObjectAlreadyInCollectionException
        else:
            raise TypeError

which basically only holds a list and can add a ClientDAO to it.

And the following, which derives from it:

class ClientFileRepository(ClientRepo):

    def __init__(self, fileName) -> None:
        super().__init__()
        self.__fileName = fileName
        self.__file = None

    def hasClientWithId(self, clientId):
        self.__loadRepo()
        hasClientWithId = super().hasClientWithId(clientId)
        super().clean()
        return hasClientWithId

    def addClient(self, client):
        self.__loadRepo()
        super().addClient(client)
        self.__storeRepo()
        super().clean()

    def __loadFileReadMode(self):
        self.__file = open(self.__fileName, "r")

    def __loadFileWriteMode(self):
        self.__file = open(self.__fileName, "w")

    def __closeFile(self):
        self.__file.close()

    def __loadRepo(self):
        self.__loadFileReadMode()
        for line in self.__file:
            splitLine = line.split()
            clientToAdd = ClientDAO(splitLine[1])
            clientToAdd.setClientId(int(splitLine[0]))
            super().addClientWithId(clientToAdd)
        self.__closeFile()

    def __storeRepo(self):
        self.__loadFileWriteMode()
        self.__file.write("")
        for client in super().getList():
            self.__file.write(self.clientToString(client))
        self.__closeFile()

    def clientToString(self, clientDAO):
        return str(clientDAO.getId()) + " " + clientDAO.getName() + "\n"

a class which should load the list from a file, call addClient from parent, and store the updated list in the file. The problem is that after child class loads the file in addClient, it calls the method in the parent, which calls hasClientWithId, from the child, again. But I want it to call hasClientWithId, from the parent, that is, the context it is in. Can I achieve that?

Upvotes: 0

Views: 234

Answers (1)

BurningKarl
BurningKarl

Reputation: 1196

I can think of several ways to achieve your goal. I ranked them from worst to best

1. Exactly what you asked for

You wanted that ClientRepo.addClient calls ClientRepo.hasClientWithId instead of ClientFileRepository.hasClientWithId. It is possible to enforce that:

class ClientRepo(Repository): 
    def addClient(self, client): 
        if type(client).__name__ == 'ClientDAO': 
            if not ClientRepo.hasClientWithId(self, client.getId()): 
                client.setClientId(self.__maximumIndexInClientList() + 1)
                self.__clientList.append(client) 
            else: 
                raise ObjectAlreadyInCollectionException
        else: 
            raise TypeError

This is not a good approach, because it's unintuitive and breaks the principles of OOP. Any other programmer writing a subclass of ClientRepo that overrides hasClientWithId would expect that this will have an effect for every call to hasClientWithId even inside of addClient

2. Let ClientFileRepository decide which function to use

Add a variable

self.__isFileOpen = False

in ClientFileRepository.__init__, set it to True when you open the file and to False when you close the file. Then change the hasClientWithId within ClientFileRepository to

def hasClientWithId(self, clientId):
    if not self.__isFileOpen:
        self.__loadRepo()
        result = super().hasClientWithId(clientId)
        super().clean() 
        return result
    else:
        return super().hasClientWithId(clientId)

to avoid opening the same file again. This works, but it is pretty difficult to write new functions for this class, because you always need to be aware if the function call is a call from within your class or from somewhere else. Also this seems pretty inefficient, because you read and write the entire file, even when you only add one client.

3. Read the file only once and modify the underlying ClientRepo

class ClientFileRepository(ClientRepo): 
    def __init__(self, fileName) -> None: 
        super().__init__() 
        self.__fileName = fileName 
        self.__loadRepo()

    # No hasClientWithId needed

    def addClient(self, client):
        super().addClient(client)
        self.__storeRepo()

    def __loadRepo(self):
        with open(self.__filename) as file:
            for line in file: 
                splitLine = line.split() 
                clientToAdd = ClientDAO(splitLine[1])
                clientToAdd.setClientId(int(splitLine[0])) 
                super().addClientWithId(clientToAdd) 

    def __storeRepo(self): 
        with open(self.__filename, "w") as file:
            file.write("") 
            for client in super().getList():
                file.write(self.clientToString(client))

This obviously assumes that the file is not changed by someone else between calls to addClient and the program still overwrites the entire file for every addClient. If this is a problem for you it is best to be explicit and make loadRepo and storeRepo public. Then the programmer using this class can decide when loading and saving are necessary and useful. You can use context managers for this.

Extra: Read and save the file for every method

You can use function decorators to use solution 2 without writing the same code for every function:

import functools

def loadAndStore(function):
    @functoools.wraps(function)
    def wrappedFunction(self, *args, **kwargs):
        if self.__isFileOpen:
            return function(self, *args, **kwargs)
        else:
            self.__isFileOpen = True
            self.__loadRepo()
            try:
                return function(self, *args, **kwargs)
            except Exception as e: # Only catch expected exceptions
                raise
            finally:
                self.__storeRepo()
                self.clear() # some cleanup
                self.__isFileOpen = False
    return wrappedFunction

class ClientFileRepository(ClientRepo): 
    def __init__(self, fileName) -> None: 
        super().__init__() 
        self.__fileName = fileName
        self.__isFileOpen = False

    @loadAndStore
    def hasClientWithId(self, clientId):
        return super().hasClientWithId(clientId)

    @loadAndStore
    def addClient(self, client):
        super().addClient(client)

    def __loadRepo(self):
        with open(self.__filename) as file:
            for line in file: 
                splitLine = line.split() 
                clientToAdd = ClientDAO(splitLine[1])
                clientToAdd.setClientId(int(splitLine[0])) 
                super().addClientWithId(clientToAdd) 

    def __storeRepo(self): 
        with open(self.__filename, "w") as file:
            file.write("") 
            for client in super().getList():
                file.write(self.clientToString(client))

Be careful here, using this is not very intuitive. For example self.__isFileOpen is defined in __init__, but none of the methods below directly use it. Instead its use is hidden in the loadAndStore decorator.

Some quick hints at the end:

  • type(client).__name__ == 'ClientDAO' is bad practice. Use isinstance(client, ClientDAO) to fully adopt OOP
  • If this is not part of a bigger project with given naming conventions use the python style guide
  • Using private variables like __fileName is generally considered unnecessary, just prefix the variable with one underscore to indicate "internal use". The same is true for functions.

Upvotes: 1

Related Questions