Results 1 to 16 of 16
  1. #1
    gcampton Guest

    Default passing vs accessor

    Just wondering which would be the better option in this example. I've created a standard Card and Deck class and I'm not sure which the better option would be to either use the getter or pass the arraylist by reference.
    I have opted for using the getDeck(); in populateDeck() and shuffleDeck()
    here's my code:

    Deck.java
    Java Code:
    import java.util.ArrayList;
    import java.util.Collections;
    
    public class Deck
    {
    	private ArrayList<Cards> aDeck;
    	
    	public Deck(ArrayList<Cards> deck)
    	{
    		setDeck(deck);
    		populateDeck();
    	}
    	
    	private void setDeck (ArrayList<Cards> aDeck)
    	{
    		this.aDeck = aDeck;
    	}
    	
    	public ArrayList<Cards> getDeck()
    	{
    		return aDeck;
    	}
    	
    	public void populateDeck()
    	{
    		for (Suit suit : Suit.values())
                        for (Value value : Value.values())
             	        getDeck().add(new Cards(suit,value));
    	}
    	
    	public void shuffleDeck()
    	{
    		Collections.shuffle(getDeck());
    	}
    }
    Main.java
    Java Code:
    public static void main (String[] args)
    {
    	ArrayList<Cards> deck1 = new ArrayList<Cards>(52);
    	Deck deck = new Deck(deck1);
    	deck1=deck.getDeck();
                ...
    Last edited by gcampton; 01-06-2010 at 03:09 PM.

  2. #2
    JosAH's Avatar
    JosAH is offline Moderator
    Join Date
    Sep 2008
    Location
    Voorschoten, the Netherlands
    Posts
    13,785
    Blog Entries
    7
    Rep Power
    21

    Default

    Don't pass that entire ArrayList to the entire world; that makes is possible to do getDeck().clear() with possible destastrous results. Create a few other getter methods such as getCard(int rank) and getNofCards(); that way your Deck class protects the ArrayList (why not use the interface List instead?) from the world. Also, create that List/ArrayList in the constructor of the Deck.

    kind regards,

    Jos

  3. #3
    gcampton Guest

    Default

    Quote Originally Posted by JosAH View Post
    Don't pass that entire ArrayList to the entire world; that makes is possible to do getDeck().clear() with possible destastrous results.
    In the real world yes, but is this not data hiding anyway? I thought that was the point of mutators and accessors.

    Quote Originally Posted by JosAH View Post
    Create a few other getter methods such as getCard(int rank) and getNofCards(); that way your Deck class protects the ArrayList (why not use the interface List instead?) from the world. Also, create that List/ArrayList in the constructor of the Deck.

    kind regards,

    Jos
    1. I'm not quite sure what you have in mind for those methods and how this would get around the getDeck() method, but I can easily get the current deck count by using the size(); As my idea was as I dealt cards I would use removeRange(int fromIndex=0, int toIndex=1)...
    just an idea at the moment.

    2. What difference or advantage would be in using the List interface? I thought they were pretty much the same.

    3. If I create in constructor,( I'm guessing you mean instead of Main).. then I only have 1 Deck, I thought it would be better this way so I could create multiple decks if I wished. And just create the arraylists in the Main or eventually in the game class. eg. 21, poker, memory classes etc...

  4. #4
    gcampton Guest

    Default

    I'm not quite sure what you have in mind for those methods and how this would get around the getDeck() method
    Oh, you mean instead of... get rid of getDeck()?

    I guess I could put in the constructor some type of int variable, for the number of decks(lists) to create or something... I dunno how this could happen, unless it was between a set number like 1-6.

  5. #5
    Tolls is offline Moderator
    Join Date
    Apr 2009
    Posts
    12,224
    Rep Power
    20

    Default

    For interfaces, it is better to code against the interface, not the concrete class. This allows you to change the implementation without affecting any users of your class.

    As for the deck, it is a deck of cards, so presumably it can set itself up (its List<Card>) and doesn't need them provided.

    And I agree that you shouldn't be returning the List anyway. Decide what you can do with your Deck and provide methods for that. Off the top of my head I can think of shuffle(), deal(int numberOfCards), nextCard(), returnCard(Card), maybe some sort of reset()? In fact, turn that into an interface, and then you can have implementations of it, like StandardDeck (52 cards), ShortDeck (there would be different types of these for different games).

    The use of a Deck does not need access to the List<Card>.

  6. #6
    JosAH's Avatar
    JosAH is offline Moderator
    Join Date
    Sep 2008
    Location
    Voorschoten, the Netherlands
    Posts
    13,785
    Blog Entries
    7
    Rep Power
    21

    Default

    Quote Originally Posted by gcampton View Post
    In the real world yes, but is this not data hiding anyway? I thought that was the point of mutators and accessors.
    Indeed you're not hiding any data at all, some loony can clear that ArrayList you are exposing to the world and the results would be unpredictable. You are even exposing the concrete class (ArrayList). The methods I suggested expose a single (immutable?) card in the deck and the other returns the number of cards in the deck; all that something else should know about your deck. Abstract away more than you did above; it pays back in the end.

    kind regards,

    Jos

  7. #7
    gcampton Guest

    Default

    Thanks guys.

  8. #8
    gcampton Guest

    Default

    Also 1 more thing, what's advantage to using List?

  9. #9
    collin389 is offline Senior Member
    Join Date
    Nov 2009
    Posts
    235
    Rep Power
    6

    Default

    Here is a way to deal a hand:
    Java Code:
    int n = //Number of cards to be delt
    List<Card> handView = deck.subList(deckSize - n, deckSize);
    List<Card> hand = new ArrayList<Card>(handView);
    Once you shuffle the deck, you can make a sub list which automatically takes the card out of the deck. It does take it from the bottom, but that shouldn't really matter because it is shuffled anyway. Also, I have a list of Card, a separate class.

  10. #10
    gcampton Guest

    Default

    Ah ok, so pretty much the same idea as i had but I was going about it a silly way.

    So wouldn't a Vector also be a good idea in this?
    Last edited by gcampton; 01-07-2010 at 03:42 AM.

  11. #11
    collin389 is offline Senior Member
    Join Date
    Nov 2009
    Posts
    235
    Rep Power
    6

    Default

    Its up to you but a I don't really know how they differ besides the fact that a Vector is synchronized and an ArrayList is not.

  12. #12
    JosAH's Avatar
    JosAH is offline Moderator
    Join Date
    Sep 2008
    Location
    Voorschoten, the Netherlands
    Posts
    13,785
    Blog Entries
    7
    Rep Power
    21

    Default

    Quote Originally Posted by gcampton View Post
    Also 1 more thing, what's advantage to using List?
    Suppose you want to run that Deck class in a multithreaded application. You'd need a Vector instead of an ArrayList; also suppose that a lot of people/companies use your code already; in your scenario you're stuck; if you'd used a List in the API instead you'd be safe to use a Vector in your class while all your users don't have to change a single bit to keep their programs running.

    kind regards,

    Jos

  13. #13
    Tolls is offline Moderator
    Join Date
    Apr 2009
    Posts
    12,224
    Rep Power
    20

    Default

    Re: Vector.
    It's old, Java 1.1, and really ought to be viewed as defunct (bar for Java ME, apparently). Anything Vector can do, something in Collections can do just as well, if not better, including synchronisation.

    As for "why List", as I said before, code to interfaces. The code using your List only cares about the functionality provided, not how that functionality is actually coded. This allows you (as Jos says) to change the how without affecting the other code at all. It means you can, if it turns out to (maybe) provide better performance, change from an ArrayList to a LinkedList...outside of Deck no one need know this.

  14. #14
    gcampton Guest

    Default

    Ah ok, I thought from reading the API that Vector was pretty much the same as list, anyway. I changed Deck to an interface, but I feel I will have a lot of repeat code doing this (in classes implementing). I think perhaps abstract is the way to go, but it still has the same problem not quite as bad.

    I did devise a way for a regular Deck class to take any number of decks and append them to the List, shuffle etc. which is the most reduced code and allows for any number of decks up to INT_MAX. This was before the above conversation however.

    Java Code:
    public interface Deck 
    {
        // For each suit, rank create a unique card
        void populateDeck();
        // shuffles deck/s to randomise cards
        void shuffleDeck();
        // returns a single card
        List<Card> dealCard();
        int getNumOfCards();
        // returns 5
        List<Card> dealHand(); 
    }
    or

    Java Code:
    import java.util.ArrayList;
    import java.util.Collections;
    import java.util.List;
    
    public class Deck
    {
    	private List<Cards> aDeck;
    	private int numOfDecks;
    	/**
    	 * Default Constructor for Deck.
    	 */
    	public Deck(int numOfDecks)
    	{
                    if (numOfDecks<1)
                                 numOfDecks=1;
    		aDeck = new ArrayList<Cards>(52);
    		populateDeck(aDeck);
    		setNumOfDecks(numOfDecks);
    		shuffleDeck();
    	}
    	/**
    	 * Populates the Deck with 52 unique Cards.
    	 */
    	private void populateDeck(List<Cards> newDeck)
    	{
    	         for (Suit suit : Suit.values())
                             for (Value value : Value.values())
             	                newDeck.add(new Cards(suit,value));
    	}
    	/**
    	 * Shuffles the Deck randomising the Cards order.
    	 */
    	public void shuffleDeck()
    	{
    		Collections.shuffle(aDeck);
    	}
    	
    	public List<Card> dealCard()
    	{
    		return aCard; // FIXME
    	}
    
    	public List<Card> dealHand(){  }  // FIXME
    
    	public int getNumOfCards()
    	{
    		return aDeck.size();
    	}
    	
    	public void printRemainingDeck()
    	{
    		for (int i=0; i<aDeck.size(); i++)
    			System.out.println(aDeck.get(i).toString());
    	}
    	/**
    	 * @param numOfDecks the numOfDecks to set
    	 */
    	private void setNumOfDecks (int numOfDecks)
    	{
    		this.numOfDecks = numOfDecks;
    		for (int i=0; i<getNumOfDecks(); i++)
    		{
    			List<Cards> aNewDeck = new ArrayList<Cards>(52);
    			populateDeck(aNewDeck);
    			aDeck.addAll(aNewDeck);
    			aNewDeck.clear();
    		}
    	}
    	/**
    	 * @return the numOfDecks
    	 */
    	public int getNumOfDecks ()
    	{
    		return numOfDecks;
    	}
    }
    EDIT: I think due to being so many number of deck possibilities eg. poker 1-2 (depending on game), twentyone 2-6, canaster samba 4-8, etc. I'll just do abstract, only need to change a couple methods per class.
    Last edited by gcampton; 01-08-2010 at 04:07 AM.

  15. #15
    collin389 is offline Senior Member
    Join Date
    Nov 2009
    Posts
    235
    Rep Power
    6

    Default

    Hmm, it seems you using getNumDecks and setNumDecks is wasting a lot of code. Unless you need getNumDecks for something else, you should delete both of those methods. Then change your constructor to:
    Java Code:
    public Deck(int numOfDecks)
    	{
                    if (numOfDecks<1)
                                 numOfDecks=1;
    		aDeck = new ArrayList<Cards>(52*numOfDecks);
    		populateDeck(aDeck);
    		shuffleDeck();
    	}
    and change populateDeck() to
    Java Code:
    private void populateDeck(List<Cards> newDeck)
    	{
                for(int i=0; i<numOfDecks; i++)
    	         for (Suit suit : Suit.values())
                             for (Value value : Value.values())
             	                newDeck.add(new Cards(suit,value));
                                         
    	}
    It seems like this would save a lot of code.

  16. #16
    gcampton Guest

    Default

    true :) but that's not very abstract.

    IMO I don't particularly see what is wrong with the 1 class, it does pretty much everything an abstraction set could achieve. And as it does everything, not allowing people to make child classes doesn't do much harm, unless in the case of wanting to add more methods.

    I think pretty much the only case I can think of is wanting to add values not enum values, but integer values to the ranks, eg ace-jack 10, 10-ace corresponding value. but then in the case of other games ace-jack can have different values. It's been a little over 18 years since i've played canasta or samba but I'm pretty sure the face cards had different values. (maybe I'm thinking of points)
    edit: even then this can be added to Card Class... Doesn't particularly need to be in Deck.

Similar Threads

  1. Passing value....
    By casid in forum JavaServer Pages (JSP) and JSTL
    Replies: 1
    Last Post: 01-03-2010, 12:19 PM
  2. passing something
    By dinosoep in forum Threads and Synchronization
    Replies: 2
    Last Post: 12-05-2009, 10:26 AM
  3. Accessor method
    By DC200 in forum New To Java
    Replies: 19
    Last Post: 06-17-2009, 10:11 AM
  4. Accessor help needed (I think...- little confussed)
    By thomase in forum New To Java
    Replies: 7
    Last Post: 03-11-2009, 11:29 AM
  5. Replies: 6
    Last Post: 12-03-2008, 12:15 AM

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •