Constructor problem

Discussion in 'Python' started by Nick, Oct 28, 2004.

  1. Nick

    Nick Guest

    I'm writing a simple class to represent a deck of cards. Here is the
    stripped down version. Notice classes Deck and Deck2. Class Deck works,
    Deck2 does not. My thinking is that it should be more efficient (a la
    Deck2) to pre-create the list, and then modify it, rather than appending
    Cards as in Deck.

    # --------------------snip--------------
    class Card:
    def __init__(self, rank=0, suit=0):
    self.rank=rank
    self.suit=suit

    def __repr__(self):
    r = "A23456789TJQK"[self.rank]
    s = "SHCD"[self.suit]
    return r+s


    class Deck:
    def __init__(self):
    self.deck = []
    for r in range(13):
    for s in range(4):
    self.deck.append(Card(r,s))

    class Deck2:
    def __init__(self):
    self.deck = 52 * [Card()]
    n = 0
    for r in range(13):
    for s in range(4):
    self.deck[n].rank = r
    self.deck[n].suit = s
    n = n + 1


    a = Deck()
    print "A --> ", a.deck
    b = Deck2()
    print "B --> ", b.deck
    # ------------snap---------------------

    Deck A looks right, 52 cards, one of each. Deck B, on the other hand, is
    52 of the same card, all rank 12, suit 3.

    What am I missing???

    Thanks!
    Nick.
     
    Nick, Oct 28, 2004
    #1
    1. Advertising

  2. Nick

    Robert Kern Guest

    Nick wrote:

    > class Deck2:
    > def __init__(self):
    > self.deck = 52 * [Card()]


    Here you are creating a single Card instance and putting the same object
    into a list 52 times. Each slot in the list points to the same object.

    Another lesson to take away from this: Don't optimize prematurely. The
    simplest implementation should be fine unless proven otherwise via
    profiling.

    --
    Robert Kern


    "In the fields of hell where the grass grows high
    Are the graves of dreams allowed to die."
    -- Richard Harter
     
    Robert Kern, Oct 28, 2004
    #2
    1. Advertising

  3. "Nick" <> wrote in message
    news:418169ec$...
    > I'm writing a simple class to represent a deck of cards. Here is the
    > stripped down version. Notice classes Deck and Deck2. Class Deck works,
    > Deck2 does not. My thinking is that it should be more efficient (a la
    > Deck2) to pre-create the list, and then modify it, rather than appending


    I strongly suspect premature optimization here.

    > Cards as in Deck.
    >
    > # --------------------snip--------------
    > class Card:
    > def __init__(self, rank=0, suit=0):
    > self.rank=rank
    > self.suit=suit
    >
    > def __repr__(self):
    > r = "A23456789TJQK"[self.rank]
    > s = "SHCD"[self.suit]
    > return r+s
    >
    >
    > class Deck:
    > def __init__(self):
    > self.deck = []
    > for r in range(13):
    > for s in range(4):
    > self.deck.append(Card(r,s))
    >
    > class Deck2:
    > def __init__(self):
    > self.deck = 52 * [Card()]


    This creates a list of 52 references to the card created by Card().

    > n = 0
    > for r in range(13):
    > for s in range(4):
    > self.deck[n].rank = r
    > self.deck[n].suit = s
    > n = n + 1
    >
    >
    > a = Deck()
    > print "A --> ", a.deck
    > b = Deck2()
    > print "B --> ", b.deck
    > # ------------snap---------------------
    >
    > Deck A looks right, 52 cards, one of each. Deck B, on the other hand, is
    > 52 of the same card, all rank 12, suit 3.
    >
    > What am I missing???
    >
    > Thanks!
    > Nick.


    How about a list comprehension?

    class Deck3:
    def __init__(self):
    self.deck = [Card(r, s) for s in range(4) for s in range(13)]
     
    Daniel Fackrell, Oct 28, 2004
    #3
  4. Nick

    Larry Bates Guest

    Nick wrote:
    > I'm writing a simple class to represent a deck of cards. Here is the
    > stripped down version. Notice classes Deck and Deck2. Class Deck works,
    > Deck2 does not. My thinking is that it should be more efficient (a la
    > Deck2) to pre-create the list, and then modify it, rather than appending
    > Cards as in Deck.
    >
    > # --------------------snip--------------
    > class Card:
    > def __init__(self, rank=0, suit=0):
    > self.rank=rank
    > self.suit=suit
    >
    > def __repr__(self):
    > r = "A23456789TJQK"[self.rank]
    > s = "SHCD"[self.suit]
    > return r+s
    >
    >
    > class Deck:
    > def __init__(self):
    > self.deck = []
    > for r in range(13):
    > for s in range(4):
    > self.deck.append(Card(r,s))
    >
    > class Deck2:
    > def __init__(self):
    > self.deck = 52 * [Card()]
    > n = 0
    > for r in range(13):
    > for s in range(4):
    > self.deck[n].rank = r
    > self.deck[n].suit = s
    > n = n + 1
    >
    >
    > a = Deck()
    > print "A --> ", a.deck
    > b = Deck2()
    > print "B --> ", b.deck
    > # ------------snap---------------------
    >
    > Deck A looks right, 52 cards, one of each. Deck B, on the other hand, is
    > 52 of the same card, all rank 12, suit 3.
    >
    > What am I missing???
    >
    > Thanks!
    > Nick.


    self.deck = 52 * [Card()]

    Problem is with this line. It doesn't do what you want.

    Try this from interpreter prompt:

    class card:
    pass

    deck=52*[card()]
    id(deck[0])
    id(deck[1])
    ....

    You will notice that all 52 entries in the list point
    to the same instance of the card class.

    As far as performance is concerned you are doing what
    is commonly called as premature optimization. I set
    up a loop and complete deck creation takes only 0.00015
    seconds (that's 0.15 milliseconds) on my machine. I
    wouldn't worry about optimizing this part of the program.

    -Larry
     
    Larry Bates, Oct 28, 2004
    #4
  5. Nick <> wrote:
    ...
    > I'm writing a simple class to represent a deck of cards. Here is the
    > stripped down version. Notice classes Deck and Deck2. Class Deck works,
    > Deck2 does not. My thinking is that it should be more efficient (a la
    > Deck2) to pre-create the list, and then modify it, rather than appending
    > Cards as in Deck.


    Everybody's accusing you of premature optimization, and they have a
    point, but, there IS a very simple approach that's fast and useful
    (particularly in Python 2.4 -- if you're interested in performance, get
    and use 2.4 beta 1 *NOW*!!!-): a list comprehension:

    self.deck = [Card(r, s) for r in xrange(13) for s in xrange(4)]


    Alex
     
    Alex Martelli, Oct 29, 2004
    #5
  6. Nick

    Nick Guest

    Nick wrote:
    > I'm writing a simple class to represent a deck of cards. Here is the
    > stripped down version. Notice classes Deck and Deck2. Class Deck works,
    > Deck2 does not. My thinking is that it should be more efficient (a la
    > Deck2) to pre-create the list, and then modify it, rather than appending
    > Cards as in Deck.
    >

    Thanks for the replies... I smack my head in dismay.
     
    Nick, Oct 29, 2004
    #6
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. Giulio
    Replies:
    9
    Views:
    1,066
    Patrick Kowalzick
    Jun 25, 2003
  2. Brett Irving
    Replies:
    3
    Views:
    3,350
    John Harrison
    Jun 29, 2003
  3. lallous
    Replies:
    5
    Views:
    8,870
    David Harmon
    Jan 23, 2004
  4. Aire
    Replies:
    3
    Views:
    486
    Mike Wahler
    Jan 25, 2004
  5. Generic Usenet Account
    Replies:
    10
    Views:
    2,340
Loading...

Share This Page