Problem with Inheritance

[KronikAlkoholik]

I'm creating a deck and card class for a program I'm doing at school. I'm very new to Java so some simple things are standing in my way. This may not be the best solution and you might now a better one but I really wan't to understand why this is happening.

First I created a Card class (note all classes are in icelandic so it might not make much sense so I'll try to explain)

package is.hr.spider;

public class Spil {

            /*
spil means Card and the enum gildi is for the values AS = ACE an so forth.
*/

	public enum Gildi {
		AS, TVISTUR, THRISTUR, FJARKI, FIMMA, SEXA,
		SJOA, ATTA, NIA, TIA, GOSI, DROTTNING, KONGUR
	}

           //Sort is for suit and it's Heart, Spade and so on.

	public enum Sort {
		HJARTA, SPADA, TIGUL, LAUFA
	}

            // the data fields of card class, that is the suit and the value
	
	private Sort sort;
	private Gildi gildi;

             // the constructor with no args
	
	public Spil() {
		
	}

             /*
constructor with args, veljaGildi and veljaSort methods are to set the right enums in the for loop in the deck class
*/
	
	public Spil(int s, int g)
	{
		this.veljaGildi(g);
		this.veljaSort(s);
	}
	
	public void veljaGildi(int i)
	{
		switch (i) {
		case 1: this.gildi = Gildi.AS;
		break;
		case 2: this.gildi = Gildi.TVISTUR;
		break;
		case 3: this.gildi = Gildi.THRISTUR;
		break;
		case 4: this.gildi = Gildi.FJARKI;
		break;
		case 5: this.gildi = Gildi.FIMMA;
		break;
		case 6: this.gildi = Gildi.SEXA;
		break;
		case 7: this.gildi = Gildi.SJOA;
		break;
		case 8: this.gildi = Gildi.ATTA;
		break;
		case 9: this.gildi = Gildi.NIA;
		break;
		case 10: this.gildi = Gildi.TIA;
		break;
		case 11: this.gildi = Gildi.GOSI;
		break;
		case 12: this.gildi = Gildi.DROTTNING;
		break;
		case 13: this.gildi = Gildi.KONGUR;
		break;
		}
	}
	
	public void veljaSort(int i)
	{
		switch (i) {
		case 0: this.sort = Sort.HJARTA;
		break;
		case 1: this.sort = Sort.SPADA;
		break;
		case 2: this.sort = Sort.TIGUL;
		break;
		case 3: this.sort = Sort.LAUFA;
		break;
		}
	}
	
	public void setGildi(Gildi gildi)
	{
		this.gildi = gildi;
	}
	
	public void setSort(Sort sort)
	{
		this.sort = sort;
	}
	
            //method to return the type of card as a string

	public String getSpilStrengur()
	{
	return sort + "" + gildi;
	}
	
}

Ok next is the Deck class

package is.hr.spider;

import java.util.Arrays;
import java.util.Collections;

// stokkur means Deck

public class Stokkur {
	
	// data fields, array of cards, and a integer value
	//to keep track how many cards have been drawn
	
	private Spil[] stokkur;
	private int spilDreginn = 0;
	
	//The constructor that uses loops to set all cards in the deck
	
	public Stokkur()
	{
		stokkur = new Spil[52];
		for (int i=0; i<4; i++)
		{
			for (int y=0; y<13; y++)
			{
				int s = (i)*13+y;
				Spil sp = new Spil();
				sp.veljaSort(i);
				sp.veljaGildi(y+1);
				stokkur[s] = sp;
			}
		}
		this.stokka();
	}
	
	// method to draw cards, start at Deck[51] and down
	
 	public Spil draga()
	{
		int i = (this.stokkur.length - 1) - spilDreginn;
		spilDreginn++;
		return stokkur[i];
	}
 	
 	//method to shuffle
	
	public void stokka()
	{
		Collections.shuffle(Arrays.asList(stokkur));
	}
}

OK I've ran this and it seems to work perfectly

Then I created a new Deck class the inherits the other one. The data fields are private so I have to write them again to the new class to use them right?

ok the I create a new constructor, this one makes a double deck.

I then make a main class and try to run the new deck and draw all the cards there, what I get is an Illegal out of bounds array, it's like the draw and shuffle methods work but only for the old class or something.

package is.hr.spider;


public class SpiderStokkur extends Stokkur {
	
	private Spil[] stokkur;
	private int spilDreginn = 0;
	
	public SpiderStokkur()
	{
		stokkur = new Spil[104];
		for (int i=0; i<4; i++)
		{
		for (int y=0; y<13; y++)
			{
				int s = (i)*13+y;
				Spil sp = new Spil();
				sp.veljaSort(i);
				sp.veljaGildi(y+1);
				stokkur[s] = sp;
				stokkur[(s+52)] = sp;
			}
		}
		this.stokka();
	}
	
}

package is.hr.spider;

public class Test {

	
	public static void main(String[] args) {
		
		SpiderStokkur s = new SpiderStokkur();
		Spil sp;
		for (int i = 1; i<=104; i++)
		{
			sp = s.draga();
			System.out.println(sp.getSpilStrengur());
		}

	}

}

this is what the console returns:

LAUFAAS
TIGULNIA
LAUFAKONGUR
SPADASJOA
LAUFAFJARKI
LAUFANIA
HJARTANIA
LAUFAGOSI
SPADAAS
HJARTAKONGUR
LAUFASEXA
SPADAATTA
SPADATHRISTUR
TIGULKONGUR
TIGULFIMMA
TIGULSEXA
TIGULTHRISTUR
SPADATVISTUR
LAUFAATTA
TIGULDROTTNING
SPADADROTTNING
LAUFATHRISTUR
HJARTASJOA
SPADAFIMMA
SPADATIA
HJARTAGOSI
HJARTATVISTUR
TIGULTVISTUR
SPADASEXA
SPADANIA
TIGULGOSI
SPADAFJARKI
TIGULATTA
LAUFATIA
LAUFADROTTNING
HJARTATIA
HJARTASEXA
HJARTADROTTNING
SPADAGOSI
HJARTAAS
TIGULAS
HJARTATHRISTUR
LAUFAFIMMA
TIGULTIA
TIGULSJOA
LAUFATVISTUR
LAUFASJOA
HJARTAFIMMA
TIGULFJARKI
HJARTAFJARKI
SPADAKONGUR
HJARTAATTA
Exception in thread "main" java.lang.ArrayIndexOutOfBoundsException: -1
at is.hr.spider.Stokkur.draga(Stokkur.java:41)
at is.hr.spider.Test.main(Test.java:13)

He doesn't seem to create an array of more then 52, why I don't know. It's like he's still running the old class. Why would he do that.

[Fubarable]

When your Test class calls the draga() method on s, your SpiderStokkur object, it is calling a method of SpiderStokkur's parent class, Stokkur. Since the parent's array only has 52 cards, it runs out of cards half way through this deal.

I think that this is not a good example of inheritance, that you can't just create a DoubleDeck from your current Deck and make it work. Better perhaps is to have your original Deck or Stokkur class use an ArrayList to hold the Cards or Spils, and to perhaps give your Stokkur class another constructor, one that accepts an int as a parameter. If the int is 2, you use two decks, if 1, then the default single deck.

[Fubarable]

A couple of other suggestions:

1) I would have the Spil constructor accept one each of your enums, Gildi and Sort as parameters:

  public Spil(Sort sort, Gildi gildi) {
    this.sort = sort;
    this.gildi = gildi;
  }

This would allow you to get rid of the ugly switch/case statement.

2) I would give the Spil class two getter methods, getGildi and getSort to allow other objects to query Spil and find out what kind of suit and value it has.

3) I would have Stokkur hold its cards in the more flexible and variable sized ArrayList rather than a fixed-size array.

private ArrayList<Spil> spilList = new ArrayList<Spil>();

4) When creating a deck, I'd use a for-each loop to loop through the Gildi and Sort enums. You can extract an array of the enum items with the enum's values() method. For instance
Sort.values() will return {Sort.HJARTA, Sort.SPADA, Sort.TIGUL, Sort.LAUFA}

5) I would give Stokkur a constructor that takes an int parameter that stands for the number of decks it should hold.
something like so:

  public Stokkur(int talaAfStokkur) {
    for (int i = 0; i < talaAfStokkur; i++) {
      // foreach works well here
      // Gildi.values() returns an array of the Gildi enum values
      for (Gildi gildi : Gildi.values()) {
        for (Sort sort : Sort.values()) {
          Spil sp = new Spil(sort, gildi);
          spilList.add(sp);
        }
      }
    }
    this.stokka();
  }

6) The default constructor can simply call the constructor above:

  // default constructor
  public Stokkur() {
    this(1); // calls the constructor above with a param of 1
  }

7) I'd give Stokker an int getSize() method that returns the number of cards left in the deck.

8) The draga method can simply return spilList.remove(0) since the remove method of array list returns the item that it is removing.

9) If you give Spil a decent toString() method (you already have something similar with your getSpilStrengur method), your Test class would be simply:

  public class Test {
  
    public static void main(String[] args) {
      Stokkur s = new Stokkur(2);
      while (s.getSize() > 0) {
        Spil sp = s.draga();
        System.out.println(sp);
      }
    }
  }

[KronikAlkoholik]

A couple of other suggestions:

1) I would have the Spil constructor accept one each of your enums, Gildi and Sort as parameters:

  public Spil(Sort sort, Gildi gildi) {
    this.sort = sort;
    this.gildi = gildi;
  }

This would allow you to get rid of the ugly switch/case statement.

This is what I originally planned but couldn't get it to work with the for loop.

2) I would give the Spil class two getter methods, getGildi and getSort to allow other objects to query Spil and find out what kind of suit and value it has.

Yeah that was the plan but I did not wan't to put it in before I got it all good and working.

3) I would have Stokkur hold its cards in the more flexible and variable sized ArrayList rather than a fixed-size array.

private ArrayList<Spil> spilList = new ArrayList<Spil>();

OK the ArrayList i haven't used before so I guess I'll look it up.

4) When creating a deck, I'd use a for-each loop to loop through the Gildi and Sort enums. You can extract an array of the enum items with the enum's values() method. For instance
Sort.values() will return {Sort.HJARTA, Sort.SPADA, Sort.TIGUL, Sort.LAUFA}

5) I would give Stokkur a constructor that takes an int parameter that stands for the number of decks it should hold.
something like so:

  public Stokkur(int talaAfStokkur) {
    for (int i = 0; i < talaAfStokkur; i++) {
      // foreach works well here
      // Gildi.values() returns an array of the Gildi enum values
      for (Gildi gildi : Gildi.values()) {
        for (Sort sort : Sort.values()) {
          Spil sp = new Spil(sort, gildi);
          spilList.add(sp);
        }
      }
    }
    this.stokka();
  }

6) The default constructor can simply call the constructor above:

  // default constructor
  public Stokkur() {
    this(1); // calls the constructor above with a param of 1
  }

For each loop I don't know aboutm haven't heard about that before, guess I'll look that up. That's why I had to use the Switch statements.

7) I'd give Stokker an int getSize() method that returns the number of cards left in the deck.

8) The draga method can simply return spilList.remove(0) since the remove method of array list returns the item that it is removing.

9) If you give Spil a decent toString() method (you already have something similar with your getSpilStrengur method), your Test class would be simply:

  public class Test {
  
    public static void main(String[] args) {
      Stokkur s = new Stokkur(2);
      while (s.getSize() > 0) {
        Spil sp = s.draga();
        System.out.println(sp);
      }
    }
  }

Thank you very much for your help. This gives me much more to work form.

[Fubarable]

You're welcome, and best of luck with your project.