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)); } }
Improve this question
9
  • 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 Commented Sep 17, 2014 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. Commented Sep 17, 2014 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. Commented Sep 17, 2014 at 12:28
  • For readability's sake, please use braces with for loops and if statements as well as consistent indentation. Commented Sep 17, 2014 at 12:28
  • 5
    Your code works correctly.Please check again. Result: true true Commented Sep 17, 2014 at 12:31

1 Answer 1

Reset to default
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.

Improve this answer
Sign up to request clarification or add additional context in comments.

4 Comments

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.
I don't care about your instructor or what you told her. What I presented is the correct way to initialize those arrays.
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.
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.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.