0

Today I had this evaluation question where I had to create two classes: Dress and TestClass. I finished up those classes but when I tried to run the program I got a NullPointerException message. Here are my classes:

Class Dress:

public class Dress {
    String colors []; 
    int sizes [];

    public Dress ( String colors [], int sizes []){
       this.colors = new String [colors.length];
       this.sizes = new int [sizes.length] ;
       this.colors = colors;
       this.sizes = sizes;
    }

    public boolean search (String color){
       for (int i =0; i<colors.length;i++)
           if (colors [i].equals(color))
              return true;
       return false;
    }
    public boolean search (int size){
       for (int i =0; i<sizes.length;i++)
           if (sizes [i] == size)
              return true;
       return false;
    }
}

Class Tests:

public class Tests {
    public static void main (String args []){
       String color[] = {"Pink","Blue","Red"};
       int size[] = {8,9,7};
       Dress d = new Dress (color, size);
       System.out.println(d.search("Pink"));
       System.out.println(d.search(8));
    }
}
spongebob
  • 8,370
  • 15
  • 50
  • 83
Boshokai
  • 3
  • 1
  • 7
  • If you ask about an error that you received - please provide the error message (in your case, the exception & stacktrace), so we can help you – LionC Sep 17 '14 at 12:25
  • 6
    You need to learn about how to debug NPE's. The key is in the exception stack trace which will tell you the line number of the exception. Check the variables on that line, find the one that's null, and then check your code to see why. If you still need help, you have to post the exception stacktrace text *here* and tell *us* which line throws the exception. Note that the 1st two lines of your constructor are not necessary and are misleading so get rid of them. – Hovercraft Full Of Eels Sep 17 '14 at 12:25
  • 1
    This `this.colors = new String [colors.length];` and `this.sizes = new int [sizes.length] ;` can be removed. By the way, your code works. – Tom Sep 17 '14 at 12:28
  • For readability's sake, please use braces with for loops and if statements as well as consistent indentation. – Flo Sep 17 '14 at 12:28
  • 5
    Your code works correctly.Please check again. Result: true true – Milan Sep 17 '14 at 12:31
  • It works fine on my computer, no NullPointerException message found. – PageNotFound Sep 17 '14 at 12:31
  • It's working my system too,no exceptions arise – Selva Sep 17 '14 at 12:37
  • @Tom I tried removing this.colors = new String [colors.length]; and this.sizes = new int [sizes.length] ; and it worked, I don't know why. Previously it would point at the third line of the search method and tells me that there is a NullPointerException. – Boshokai Sep 17 '14 at 12:44
  • See http://stackoverflow.com/questions/218384/what-is-a-null-pointer-exception-and-how-do-i-fix-it – Raedwald Sep 17 '14 at 12:51

1 Answers1

1

FYI - it's a bad idea to assign a mutable reference to a private data member:

this.colors = new String [colors.length];  // The new reference is discarded after reassignment on next line
this.colors = colors;  // The program that passes this reference can modify it; changes will be visible to your class instance.

Anyone who gets ahold of that reference and changes its state will be altering your instance data member as well without regard for its private status.

Here's the right way to do it (just one for clarity):

public Dress(String [] colors) {
    if (colors == null) throw new IllegalArgumentException("colors cannot be null");
    this.colors = new String[colors.length];
    // Copy the values from the parameter array into the new, private array.
    System.arraycopy(colors, 0, this.colors, 0, this.colors.length);
}

You should always make defensive copies of private, mutable data.

duffymo
  • 305,152
  • 44
  • 369
  • 561
  • You see, the thing is, I told my instructor that the first two lines were not necessary, but she insisted on it. She told me that I should assign the value from the parameter to the array which was visible in the 3rd and 4th line in the constructor method. But for some reason it ignored the 3rd and 4th line and gave me a NullPointerException. – Boshokai Sep 17 '14 at 13:00
  • 3
    I don't care about your instructor or what you told her. What I presented is the correct way to initialize those arrays. – duffymo Sep 17 '14 at 13:08
  • What a nice way to say it, apparently you did not read past the first two lines. I asked a question, but it doesn't matter now I already found the answer. – Boshokai Sep 17 '14 at 17:39
  • Not trying to be mean; just a statement of fact. If the code still looks like what you posted it's still wrong. You'll find out someday and wonder why. – duffymo Sep 17 '14 at 17:49