4 Replies Latest reply on Jul 14, 2010 8:47 PM by 843853

    Best practice for method calling on objects within a collection.

      Hi guys

      As you may be aware, based on my other thread here. I'm designing a card game in Java. I was hoping for some advice on the best practise on how methods should be called on a custom Object contained within a custom Collection.

      I have an instance variable for the Deck class as follows:
      List<Card> deck
      When creating an instance of the class I use
      deck = new ArrayList<Card>();
      So I have a Deck which only holds Card objects. My question is, for the Card methods, should I call them on the Card objects after 'getting' the Cards from the Deck or should I write methods within the Deck class which handles this method calling. Code explanation is as follows:
      Deck standardDeck = new Deck();
      I want to retrieve the suit value of a card within the deck. Is this the best way to do it this way:
      //getCardAt is a method within the Deck class, getSuit() is a method within the Card class
      or this way:
      //getSuitForCardAt() is a method within the Deck class. This method calls the getSuit() method within its method body.
      Cheers for any help guys.

      Edited by: Faz_86 on Jul 10, 2010 9:53 AM
        • 1. LoD: Best practice for method calling on objects within a collection.
          Given that you ask this question, I would recommend to stick with the [Law of Demeter|http://en.wikipedia.org/wiki/Law_of_Demeter|explanation] ("use only one dot"). Until you gain enough experience to figure when it's OK to deviate from that law. :)

          Edited by: gnat on Jul 10, 2010 6:11 PM
          • 2. Re: Best practice for method calling on objects within a collection.
            Be sure your Card object implements hashCode() and equals(). Anything placed in a collection should implement those two methods (and likely override toString() and implement Comparable as well, though these are optional).

            WRT your specific question, I agree that the Law of Demeter is a good one. However, at the same time, it seems to me that asking the Deck simply for a Suit (which belongs to Card) is a strange situation. It has a slight design smell to me. Why retrieve the Card at index 50? From the Deck, don't you simply want the next Card available? I could see something like Deck#getSuitOfNextCard() but not really Deck#getSuitOfCardAtIndex(int). If that makes sense.

            BTW, the Law of Demeter, standing on its own does not really hold here, just my two cents. You normally want to avoid exposing getters and setters. See here and here. That is why you are seeing a long chain of dot notation. You don't really have good encapsulation yet.

            So, read those two articles, see if they make sense or not. It is normally easier to write procedurally at first (thinking it is OO). If this is the case for you, then by all means, violate the Law of Demeter. You have first already violated encapsulation. (BTW, I don't want that to come across as harsh, I wrote objects with getters and setters for over a decade until I started to value encapsulation far more than convenience).

            - Saish
            • 3. Re: Best practice for method calling on objects within a collection.
              Hey Saish

              Thanks for the response.

              My Card class does indeed override hashCode(), equals() and toString().

              The reason I am asking a card from the deck for its Suit is simply because of the rules of the game being played. The game I made is a 'Card Shredding' game where a player attempts to remove as many cards from their hand during each turn. The first to remove all their cards is the winner.
              When the game starts, two decks are created. A standard 52 card deck and an empty deck. Then 8 cards are dealt to each player and one card is dealt into the empty deck. The suit and value of the card on the empty deck called the 'shredding deck' dictates which moves are valid during each turn; The played card must match the Suit or the Value of the current card on the 'shredding deck'
              For example:
              Card on the empty deck = 8 of Spades

              The only card from a players hand which can be removed are any Spade or an Eight of any suit.

              Going back to the Deck.getSuitOfCardAtIndex(int index) , this method is needed because both the AI player and human player need to have the ability to take a look at the cards which have been added to the 'shredding deck'. Again this is because of the rules of the game. Therefore I need a method to take a look at the Suit and Value for any card in the 'shredding deck'.

              Taking all this into account, so far I have the following in my Deck class. Please comment on my design so far. As you can see I've tried to follow the Law Of Demter by creating many little wrapper methods. I understand totally wh getters and setters are bad but I cannot come up with a design solution to achieve what I need to based on the rules of the game without users getters. - Any tips on this would be great.
                   public Card dealCard()
                        Card cardToDeal = deck.remove(0);
                        return cardToDeal;
                   public void addCard(Card usedCard) //This method is used to add 'used' cards to the deck.
                   public Card getFaceCard() //Returns the current face up playing card
                        Card faceCard = deck.get(deck.size()-1);
                        return faceCard;
                   public int getFaceCardValue()
                        int faceCardValue = deck.get(deck.size()-1).getValue();
                        return faceCardValue;
                   public int getFaceCardSuit()
                        int faceCardSuit = deck.get(deck.size()-1).getSuit();
                        return faceCardSuit;
                   public String getFaceCardName()
                        String faceCardName = deck.get(deck.size()-1).toString();
                        return faceCardName;
                   public Card getCardAt(int position) //Returns the current face up playing card
                        Card card = deck.get(position);
                        return card;
                   public int getFaceCardValueAt(int position)
                        int cardValue = deck.get(position).getValue();
                        return cardValue;
                   public int getFaceCardSuitAt(int position)
                        int cardSuit = deck.get(position).getSuit();
                        return cardSuit;
                   public String getFaceCardNameAt(int position)
                        String cardName = deck.get(position).toString();
                        return cardName;
                   public int getDeckSize() //When recycling cards, the size of the deck is needed to determine the best time to add more cards.
                        return deck.size();
              • 4. Re: Best practice for method calling on objects within a collection.
                In my opinion..
                (or it's equivalent
                Card card = standardDeck.getCardAt(50);
                Suite suite = card.getSuite(); 

                ..is better since it is more logical and intuitive.
                ..mixes responsibilities - the Deck object has to be aware of properties of the cards, ie suit and value (and maybe anything else you care to add later), it is better if the Deck stuck to holding, shuffling and dealing the cards.

                A clean, well defined separation of reponsibilties is a key to designing intuitive, maintainable and extensible software.

                Hope this helps