background:
I have a model called FileInfo
, which represents a sftp file / windows dfs file.
class FileInfo(object):
def __init__(self, server_info, folder_path, file_name):
self.server_info = server_info
self.folder_path = folder_path
self.file_name = file_name
def get_file_full_path(self):
return os.path.join(self.folder_path, self.file_name)
FileInfo
has a ServerInfo
to hold the server connection info: e.g. sftp connection details.
class ServerInfo(object):
pass
class WindowsServerInfo(ServerInfo):
pass
class SFTPServerInfo(ServerInfo):
pass
this is how I print out the full path:
if __name__ == '__main__':
sftp_server_info = SFTPServerInfo()
win_server_info = WindowsServerInfo()
print FileInfo(win_server_info, r'\\dfs\domain\tmp', 'test.txt').get_file_full_path()
print FileInfo(sftp_server_info, r'/tmp', 'test.txt').get_file_full_path()
the application is running in Windows and Linux, so the output of the above code will be different -- depends on the hosting OS result on Linux
:
\\dfs\domain\tmp/test.txt
/tmp/test.txt
result on Windows
:
\\\\dfs\\domain\\tmp\\test.txt
/tmp\\test.txt
I want to have a consistent output, such as:
\\dfs\domain\tmp\test.txt
/tmp/test.txt
so I start to refactor the code:
v1. if...else...
class FileInfo(object):
def get_file_full_path(self):
if isinstance(self.server_info, SFTPServerInfo):
return r'%s/%s' % (self.folder_path, self.file_name)
elif isinstance(self.server_info, WindowsServerInfo):
return r'%s\%s' % (self.folder_path, self.file_name)
else
raise exception
it works, but if...else means it'll be a big method if I want to support more server types. so I'm going to create a dict to handle it.
v2. a dict contains ServerInfo type and function mapping
class FileInfo(object):
def get_file_full_path(self):
return get_full_path_functions[type(self.server_info)](self.folder_path, self.file_name)
def get_full_path_windows(folder_path, file_name):
return r'%s\%s' % (folder_path, file_name)
def get_full_path_sftp(folder_path, file_name):
return r'%s/%s' % (folder_path, file_name)
get_full_path_functions = {
WindowsServerInfo: get_full_path_windows,
SFTPServerInfo: get_full_path_sftp
}
it works fine, but I think it may be a good idea to move these methods into ServerInfo
v3. instance method
class FileInfo(object):
def get_file_full_path(self):
return self.server_info.get_full_path(self.folder_path, self.file_name)
class WindowsServerInfo(ServerInfo):
def get_full_path(self, folder_path, file_name):
return r'%s\%s' % (folder_path, file_name)
class SFTPServerInfo(ServerInfo):
def get_full_path(self, folder_path, file_name):
return r'%s/%s' % (folder_path, file_name)
however, get_full_path
methods are not using self
, and this method should not belongs to any instance of ServerInfo
v4. static method
class FileInfo(object):
def get_file_full_path(self):
return type(self.server_info).get_full_path(self.folder_path, self.file_name)
class WindowsServerInfo(ServerInfo):
@staticmethod
def get_full_path(folder_path, file_name):
return r'%s\%s' % (folder_path, file_name)
class SFTPServerInfo(ServerInfo):
@staticmethod
def get_full_path(folder_path, file_name):
return r'%s/%s' % (folder_path, file_name)
All these 4 versions are working as expected, please help to review them and let me know:
- from the OOP perspective, which one is the best? is there a better way to desing it?
- from Python perspective, which one is the most pythonic way?