On Thu, Apr 21, 2016, at 06:34 PM, Christopher Reimer wrote: > class PieceFactory(object): > > def factory(color, piece, position): > if piece == 'Bishop': > return Bishop(color, position) > if piece == 'King': > return King(color, position) > if piece == 'Knight': > return Knight(color, position) > if piece == 'Pawn': > return Pawn(color, position) > if piece == 'Queen': > return Queen(color, position) > if piece == 'Rook': > return Rook(color, position)
This whole section is begging for a dictionary. Like... _PIECE_TYPES= {"Bishop": Bishop, "King": King, ...] class PieceFactory(object): def factory(color, piece, position): klass = __PIECE_TYPES.get(piece) if klass is None: raise PieceException("...") return klass(color, position) Or something like that. That said, I'm not sure why its not just a function that does the same thing. Why is it in a class that only does one thing? You never even instantiate it. > def generate_set(color, pieces, positions): > for piece, position in zip(pieces, positions): > yield getattr(PieceFactory, 'factory')(color, piece, position) Whyyy are you using getattr? Something wrong with PieceFactory.factory(color, piece, position)? (Or, better yet, yield piece_factory(color, piece, position) where piece_factory is just a function) > I got the factory method from here: > http://python-3-patterns-idioms-test.readthedocs.org/en/latest/Factory.html I... that... what... I'd forget that link and pretend you never went there. Its not helpful. > Finally, VARS['VARIABLE_NAME'] got change to const['variable_name']. > Should smell better. I don't know that this changes anything about the small at all. What's the contents of this big dictionary that has everything in it for some reason? That said, dear god, 'piece' doesn't look like an english word to me anymore. I've never suffered semantic satiation from text before. --Stephen m e @ i x o k a i . i o -- https://mail.python.org/mailman/listinfo/python-list