-1

I have read a lot of posts about this and nothing I've tried seems to work. Any help would be greatly appreciated.

I have 2 Classes, ColouredShape and ShapeMatchingGame. I'm trying to create ColouredShape objects using 2 arguments and add them to an ArrayList in the ShapeMatchingGame Class. I need 4 shapes and 3 colours of each of the 4 shapes * 3, so 36 items in the array.

I can see 36 objects are adding to the array in the loop but whenever i try to access the values of the objects in the array im not seeing the expected values for the object at its index. I guess what I'm trying to ask is am I accessing the values incorrectly or have I done something wrong with creating the objects and adding them to the array?

public class ColouredShape {

static int shapeValue;
static int colourValue;
static String colour, shape;

public ColouredShape() // Default constructor
{
    this.shapeValue = 1;
    this.colourValue =1;
    this.colour ="";
    this.shape ="";
}

public ColouredShape (int shapeValue,int colourValue) // Constructor with 2 arguments
{
    this.shapeValue = shapeValue;
    this.colourValue =colourValue;
    this.colour = colour;
    this.shape =shape;
}


    public int getColour()
    {
        return colourValue; 
        }       

    public int getShape()
    {
        return shapeValue; 

        }

public class ShapeMatchingGame {

static int noShapes;
static ArrayList<ColouredShape> ColouredShapes = new ArrayList<ColouredShape>();
static int index;
static int shapeValue=1;
static int colourValue;

public static void main(String[] args)
{

    ObjectCreation();
}



    public static void ObjectCreation()
    { 
        do// runs loops 3 times to create 3 of every shape/colour combo
        {           
            do // loop to continue onto the next shape until 4 are created
            {   
                do // loop to create 3 of colours of same shape
                {       
                    colourValue++;
                    System.out.println("Shape value " +shapeValue + " colour value " +colourValue);
                    ColouredShape gameShapes = new ColouredShape(shapeValue,colourValue);
                    ColouredShapes.add(gameShapes);//creates an object of colourshapes and passes the current shapevalue + colourvalue as arguments then adds it to the arraylist
                    System.out.println ("Value of object at array index "+ index  + " shape value " + ColouredShapes.get(index).getShape()+" colour value " +ColouredShapes.get(index).getColour()+ "colour variable value = " + colourValue);                      
                    index++;
                    for (ColouredShape colouredShape : ColouredShapes) 
                    {
                        System.out.println(colouredShape.getClass().getName() + "/" +
                                colouredShape.shape + "/" +
                                colouredShape.colour);

                    }

                }while(colourValue < 3 );
                System.out.println ("Value of object at array index "+ "0"  + " shape value " + ColouredShapes.get(0).getShape()+" colour value " +ColouredShapes.get(0).getColour()+ "colour variable value = " + colourValue);
                colourValue=0;//reset colourValue to allow next iteration of the loop
                shapeValue++;//incrementing shapeValue to add colours to next shape 
                System.out.println ("Value of object at array index "+ "0"  + " shape value " + ColouredShapes.get(0).getShape()+" colour value " +ColouredShapes.get(0).getColour()+ "colour variable value = " + colourValue);
            }while(shapeValue < 5 );

            shapeValue=1; // resetting shapeValue to allow next iteration of the loop
            noShapes++;

        }while (noShapes<3);

   }

}
Paul Brinkley
  • 6,283
  • 3
  • 24
  • 33
Kieran
  • 3
  • 2
  • Why do you have both `colour` and `colourValue`? It looks like your constructor only uses 1. – 4castle Dec 14 '16 at 17:49
  • Possible duplicate of [Why does my ArrayList contain N copies of the last item added to the list?](http://stackoverflow.com/questions/19843506/why-does-my-arraylist-contain-n-copies-of-the-last-item-added-to-the-list) – 4castle Dec 14 '16 at 17:55
  • what's your output and your expected output? – isamirkhaan1 Dec 14 '16 at 18:15
  • 4castle, colour is for something else is the ColourValue class that i didnt show here, its basically for storing a string to say what the colourValue equals. Im not using that in my other class. public static String ShapeColour () // Method for returning the shape as string { colourValue = RandomColour(); if (colourValue == 1) colour = Red; if (colourValue == 2) colour = Green; if (colourValue == 3) colour = Blue; return shape; } – Kieran Dec 14 '16 at 19:04

1 Answers1

0

There are several problems on your ColouredShape:

  1. shapeValue and colourValue are static. This means there they are 'global' variables. Doesn't matter how many Colouredshapes you create, all of them will have the same values
  2. the constructor sets the value of the static variables only, and you print the values of the string text.

Change the Object to be like this:

public class ColouredShape {

    int shapeValue;
    int colourValue;

    public ColouredShape() // Default constructor
    {
        this.shapeValue = 1;
        this.colourValue = 1;
    }

    public ColouredShape(int shapeValue, int colourValue) // Constructor with 2
                                                        // arguments
    {
        this.shapeValue = shapeValue;
        this.colourValue = colourValue;
    }

    public int getColour() {
        return colourValue;
    }

    public int getShape() {
        return shapeValue;
    }
}

and the result should be as expected

Felipe Issa
  • 420
  • 1
  • 5
  • 13
  • Thanks for your reply, I have another few pieces of code in the ColourShape class that use those variables, and it flags an error if they arent set to static. what would be the best way to slove that issue? – Kieran Dec 14 '16 at 19:11
  • public static String ShapeShape() // Method for returning shape colour as string { shapeValue = RandomShape(); if (shapeValue == 1) shape = Square; if (shapeValue == 2) shape = Rectangle; if (shapeValue == 3) shape = Triangle; if (shapeValue == 4) shape = Circle; return shape; } – Kieran Dec 14 '16 at 19:14
  • This method depends on the instance data (should have a different return value for each object you create), so it cannot be static. Remove the static modifier and it should work – Felipe Issa Dec 15 '16 at 17:05