Niewłaściwe użycie __new__ do generowania klas?

Question

Tworzę klasy do radzenia sobie z nazwami plików w różnych typach udziałów plików (nfs, afp, s3, dysk lokalny). Otrzymuję jako dane wejściowe ciąg znaków identyfikujący źródło danych (np. "nfs://192.168.1.3" lub "s3://mybucket/data") itp.

Jestem podklasy konkretnych systemów plików z klasy bazowej, która ma wspólny kod. Gdzie jestem zdezorientowany jest w tworzeniu obiektu. To, co mam jest następujące:

import os 

class FileSystem(object): 
    class NoAccess(Exception): 
     pass 

    def __new__(cls,path): 
     if cls is FileSystem: 
      if path.upper().startswith('NFS://'): 
       return super(FileSystem,cls).__new__(Nfs) 
      else: 
       return super(FileSystem,cls).__new__(LocalDrive) 
     else: 
      return super(FileSystem,cls).__new__(cls,path) 

    def count_files(self): 
     raise NotImplementedError 

class Nfs(FileSystem): 
    def __init__ (self,path): 
     pass 

    def count_files(self): 
     pass 

class LocalDrive(FileSystem): 
    def __init__(self,path): 
     if not os.access(path, os.R_OK): 
      raise FileSystem.NoAccess('Cannot read directory') 
     self.path = path 

    def count_files(self): 
     return len([x for x in os.listdir(self.path) if os.path.isfile(os.path.join(self.path, x))]) 

data1 = FileSystem('nfs://192.168.1.18') 
data2 = FileSystem('/var/log') 

print type(data1) 
print type(data2) 

print data2.count_files() 

myślałem, że to będzie dobre wykorzystanie __new__ ale większość stanowisk czytałem o nim używają go zniechęcić. Czy istnieje bardziej akceptowany sposób podejścia do tego problemu?

Answer 1

Nie sądzę, używając __new__() robić to, co chcesz, jest niewłaściwe. Na przykład nie zgadzam się z zaakceptowaną odpowiedzią na tę question i że funkcje fabryczne są zawsze "najlepszym sposobem".

Jeśli naprawdę chcesz tego uniknąć, jedynymi opcjami są metaclasses lub oddzielna funkcja/metoda factory. Biorąc pod uwagę dostępne opcje, uczynienie metody __new__() - ponieważ jest ona domyślnie statyczna - jest rozsądnym podejściem.

To powiedziawszy poniżej, jest to, jak sądzę, ulepszona wersja twojego kodu. Dodałem metodę klasy i statyczną metodę narzędzia, aby pomóc w automatycznym znajdowaniu wszystkich podklas. To wspiera najważniejszy sposób, w jaki lepiej jest teraz dodawanie podklas nie wymaga modyfikacji metody __new__(). Oznacza to, że teraz można go łatwo rozszerzyć, ponieważ efektywnie ma to, co można nazwać wirtualnymi konstruktorami.

Podobna implementacja może być również wykorzystana do przeniesienia tworzenia instancji z metody __new__ do oddzielnej (statycznej) metody fabrycznej - tak w pewnym sensie przedstawiona technika jest po prostu stosunkowo prostym sposobem kodowania rozszerzalnej generycznej funkcji fabrycznej niezależnie od tego, jak się nazywa.

import os 
import re 

class FileSystem(object): 
    class NoAccess(Exception): pass 
    class Unknown(Exception): pass 

    @classmethod 
    def _get_all_subclasses(cls): 
     """ Recursively generate of all the subclasses of class cls. """ 
     for subclass in cls.__subclasses__(): 
      yield subclass 
      for subclass in subclass._get_all_subclasses(): 
       yield subclass 

    @staticmethod 
    def _get_prefix(s): 
     """ Extract any file system prefix at beginning of string s and 
      return a lowercase version of it or None when there isn't one. 
     """ 
     match = re.match(r'\s*([^:]+)://', s) # matches "xxx:" if x is any char 
     return match.group(1).lower() if match else None 

    def __new__(cls, path): 
     path_prefix = cls._get_prefix(path) 
     for subclass in cls._get_all_subclasses(): 
      if subclass.prefix == path_prefix: 
       return object.__new__(subclass , path) # using "object" avoids recursion 
     else: # no subclass with matching prefix found (or with prefix of None) 
      raise FileSystem.Unknown(
       'path "{}" has no known file system prefix'.format(path)) 

    def count_files(self): 
     raise NotImplementedError 

class Nfs(FileSystem): 
    prefix = 'nfs' 

    def __init__ (self, path): 
     pass 

    def count_files(self): 
     pass 

class LocalDrive(FileSystem): 
    prefix = None # default when no file system prefix is found 

    def __init__(self, path): 
     if not os.access(path, os.R_OK): 
      raise FileSystem.NoAccess('Cannot read directory') 
     self.path = path 

    def count_files(self): 
     return sum(os.path.isfile(os.path.join(self.path, name)) 
        for name in os.listdir(self.path)) 

data1 = FileSystem('nfs://192.168.1.18') 
data2 = FileSystem('c:/') # change as necessary for testing 

print type(data1) 
print type(data2) 

print data2.count_files() 

Answer 2

W mojej opinii, używanie w ten sposób __new__ jest naprawdę mylące dla innych osób, które mogą czytać twój kod. Również wymaga nieco hackish kodu, aby odróżnić system plików zgadywania od wprowadzania danych przez użytkownika i tworzenia Nfs i i i iz ich odpowiednimi klasami

Dlaczego nie wykonać osobnej funkcji z tym zachowaniem? Może nawet być statyczna metoda FileSystem klasy:

class FileSystem(object): 
    # other code ... 

    @staticmethod 
    def from_path(path): 
     if path.upper().startswith('NFS://'): 
      return Nfs(path) 
     else: 
      return LocalDrive(path) 

I ty nazywasz to tak:

data1 = FileSystem.from_path('nfs://192.168.1.18') 
data2 = FileSystem.from_path('/var/log')