2017-12-26 8 views
2

私は、この問題を解決するための私のアプローチが完全に間違っている可能性があるため、この質問をどのように聞くのか、それだけで答えを見つけることはできません。Pythonでは、オブジェクトのリストとやりとりするためのメソッドを提供する再利用可能なコードを書くための最良の方法は何ですか?

私はいくつかのPythonを書いている、と私は基本的には、特定のタイプ(User)のオブジェクトの数をインスタンス化し、その後、私はそれらと連携を支援する方法の数を提供するために使用されるクラス(Users)を持っていますより簡単な方法でオブジェクトを作成できます。私が持っているコードは次のようになります。

from defusedxml.ElementTree import parse 

class Users: 

    def __init__(self, path): 

     self.path = path 
     self.users = [] 

     users = parse(path).getroot() 

     for user in users: 
      u = User.user_from_xml(user) 
      self.users.append(u) 

    def __iter__(self): 

     self.i = 0 
     return self 

    def __next__(self): 

     if self.i < len(self.users): 
      self.i += 1 
      return self.users[(self.i - 1)] 
     else: 
      raise StopIteration 

    def get_user_by_id(self, user_id): 
     return next((user for user in self.users if user.id == user_id), None) 

    def search_attribute(self, attribute, value): 

     return [user for user in self.users if 
      getattr(user, attribute, None) != None and 
      value.lower() in str(getattr(user, attribute).lower())] 



class User: 

    def __init__(self, user_id, username, email, first_name, last_name): 

     self.id = int(user_id) 
     self.username = username 
     self.email = email 
     self.first_name = first_name 
     self.last_name = last_name 

    def __repr__(self): 

     if self.first_name == None or self.last_name == None: 
      return "%s (User Id: %s)" % (self.username, self.id) 

     return "%s %s (%s)" % (self.first_name, self.last_name, self.username) 

    @staticmethod 
    def user_from_xml(user): 

     return User(
      user.get("id"), 
      element.find("username").text, 
      element.find("email").text, 
      element.find("firstname").text, 
      element.find("lastname").text 
     ) 

私は、同様の方法でXMLに保存されている他のオブジェクトの数を持っている - 例えば、EventsUsersで定義されているのと同じメソッドを使用する必要があることがわかります。唯一の違いは__init__で作成されたリストに含まれるオブジェクトのタイプです。

質問があります:可読性などを維持しながら、このコードを再利用可能にするにはどうすればよいでしょうか?または、私は完全に間違ったトラックにいるかもしれません。

+0

あなたは、コンストラクタがUser、Eventsなどの間でのみ変更されると言っていますか? – ubadub

+1

あなたが何をしても、 'Users'はイテレータではありません。あなた自身のクラスを定義するか、' __iter__'をジェネレータ関数にします。または、あなたの '__iter__'と' __next__'の実装を見て、 'def __iter____(self)'を実行してください:return iter(self.users) ' –

答えて

1

これらのクラスメソッドは、本当に同じになります場合は、私が最も簡単な方法は、ちょうどその__init__メソッドで引数として別のクラス(例えば、UserまたはEventを)かかりUsersを置き換えるために、より汎用的なクラスを作ることだと思います。あなたのクラスは、そのようになります。

class Things(object): 

    def __init__(self, PATH, Thing): #Thing is a class 

     self.PATH = PATH 
     self.users = [] 

     users = parse(PATH).getroot() 

     for thing in things: 
      t = Thing.thing_from_xml(thing) 
      self.things.append(t) 

    def methods... 

より堅牢/スケーラブルなソリューションは、継承を使用することであるかもしれません。

すべてのメソッドを持つ抽象基本クラスを作成し、各子クラス内の基本クラスの__init__メソッドをオーバーライドすることができます。私はこの例を引き出すます:

class AbstractBaseClass(object): 

    def __init__(self, PATH): 
     self.PATH = PATH 
     self.things = [] 

    def methods... 



class Users(AbstractBaseClass): 

    def __init__(self, PATH): 
     super(Users, self).__init__() # calls the parent __init__ method 

     users = parse(PATH).getroot() 

     for user in users: 
      u = User.user_from_xml(user) 
      self.things.append(u) 


    #no need to define methods, as they were already defined in parent class 
    #but you can override methods or add new ones if you want 

あなたEventsクラスもAbstractBaseClassを継承し、それによってUsersの同じメソッドのすべてを持っているでしょう。あなたは継承を読むべきです、それは素晴らしいツールです。

EDITはあなたのコメントに対処するために:

プロパティが戻ってあなたのUsersクラスににその属性usersを得るための良い方法かもしれません。それがプライベートであることを示唆して_thingsthingsを変更し、そのように、usersプロパティを作成:

class Users(AbstractBaseClass): 

    @property 
    def users(self): 
     return self._things 

あなたがUsers.usersを呼び出してUsers._thingsを得ることができるこの方法を。

あなたは本当に、本当に、コードの再利用を気にしている場合、あなたも__init__に、このようなダイナミックな何かができる:

class AbstractBaseClass(object): 

    def __init__(self, PATH): 
     self._things = [] 
     self.PATH = PATH 

     setattr(self, self.__class__.__name__.lower(), self._things) 
     #This creates an attribute that is the lowercase version of the 
     #class name and assigns self._things to it 

注:私は、これは少し醜いと不要だと思います。また、同じことである2つの属性があるため、オブジェクトがインコヒーレントな状態になる可能性があります。

これは、私にはUsers.usersが冗長であるようです。私はあなたの問題の文脈を完全には認識していませんが、Usersオブジェクトはリストusersのように振る舞いますが、余分なメソッド(定義したメソッド)を使うほうがいいと思います。

AbstractBaseClassには、__iter___things属性の__iter__と定義できます。

class AbstractBaseClass(object): 

    def __init__(self, PATH): 
     self._things = [] 
     self.PATH = PATH 

    def __iter__(self): 
     return self._things.__iter__() 

    #You might also want this - it lets you do list-like indexing 
    def __getitem__(self, i): 
     return self._things.__getitem__(i) 

私は上記のは、あなたがあなたの元のコードに__iter____next__で何をしていたか、本質的に行いますが、きれいな方法で考えます。この方法では、ユーザーオブジェクトのリストを再生するために直接_thingsまたはusersにアクセスする必要はありません。 Usersクラスを使用してユーザーのリストを再生することができます。名前は、クラスの目的のようです。

+0

私は継承についても考えていましたが、私は 'Users.users'ではなく' Users.things'で終わるでしょう。言い換えれば、リストの名前は本当にそれが何を含んでいるのかを記述していません。おそらく、基底クラスを継承するクラスの名前によって推測することができます。まだ理想的ではありません。 どちらもこれまでに書いたより優れた解決策です。 –

+1

@JamesLambeth、私はあなたのコメントに対処するために私の答えを編集しました –

+0

'Users'クラスを' users'リストのように扱うことに関して優れた点があります。必ずしも 'User.things'や' User.users'を公開するのではなく、私はこのアプローチが好きです!ありがとう。 –

関連する問題