views:

225

answers:

7

Note: This is homework/assignment feel not to answer if you don't want to.

Ok after some search and reading these:

http://stackoverflow.com/questions/425439/how-to-check-if-array-element-is-null-to-avoid-nullpointerexception-in-java http://stackoverflow.com/questions/963936/gracefully-avoiding-nullpointerexception-in-java http://c2.com/cgi/wiki?NullPointerException

Am still not making any progress on how to deal with NullPointerException error on my code, snippet for questionable code:

int findElement(String element) {
          int retval = 0;

            for ( int i = 0; i < setElements.length; i++) {
               if ( setElements[i].equals(element) ) {  // This line 31  here
                  return retval = i;

               }
               else {
                   return retval = -1;
               }
           }

          return retval;
       }

       void add(String newValue) {
            int elem = findElement(newValue);
            if( numberOfElements < maxNumberOfElements && elem != -1 ) {
               setElements[numberOfElements] = newValue;
               numberOfElements++;
            } else { System.out.println("Element " + newValue + "already exist"); }
       }

It compile but adding new element to a set throws a NullPointerException error.

D:\javaprojects>java SetDemo
Enter string element to be added
A
You entered A
Exception in thread "main" java.lang.NullPointerException
        at Set.findElement(Set.java:31)
        at Set.add(Set.java:44)
        at SetDemo.main(Set.java:145)

I added another check, though honestly don't have clue if this right to line 31. if ( setElements != null && setElements[i].equals(element) ) but still no joy.

A documentation/tips or explanation is greatly appreciated.

learning, lupin

+3  A: 

Did you initialize setElements anywhere? Meaning:

String[] setElements = new String[100];

If you simply declare an array variable:

String[] setElements;

as a data member of your class it is initialized to null. You have to make it point to something. You can either do this inline:

public class MyClass {
  private String[] setElements = new String[100];
  ...
}

or in a constructor:

public class MyClass {
  private String[] setElements;

  public MyClass() {
    setElements = new String[100];
  }
  ...
}
cletus
+3  A: 

It should be setElements[i] != null && setElements[i].equals(element). If a collection contains null elements you will try to dereference a null reference when you call equals method on that element.

As for NullPointerException - you should never catch it. For things that shouldn't be null, they must be initialized properly. For those things that cannot be null - they must be checked for null before dereferencing them (i.e. calling methods on them).

The only use case for catching NullPointerException is when you are using a third-party library that you don't have the source for and has a bug that causes NullPointerException to be thrown. These cases are rare and since you only beginning to learn Java, forget that I mentioned this and concentrate on more important things.

Igor Zevaka
I think you mean "NullPointerException". NullReferenceException is a C#/.NET thing, but this question is about Java.
Jim Hurne
That is indeed what i mean. Thanks for correcting.
Igor Zevaka
+1  A: 

Try testing the element itself for null, not the array:

setElements[i] != null && setElements[i].equals(element)
Ben Hoffstein
+2  A: 

The for-loop in findElement doesn't make sense.

for ( int i = 0; i < setElements.length; i++) {
               if ( setElements[i].equals(element) ) {  // This line 31  here
                  return retval = i;

               }
               else {
                   return retval = -1;
               }
           }

You should iterate through all values before returning -1, only then do you know that there is no element in the set that matches element.

Lars Andren
+1  A: 

You should not attempt to catch a null pointer exception. Instead, the best way to avoid null pointers is:

  • In any function that takes parameters where you assume that the parameter is non-null, always check that the parameter is non-null and throw an IllegalArgumentException if it is null.
  • Whenever you invoke a function that does not allow null parameters, ensure that you do not pass a null pointer to that function; if you already know that the object is non-null (because you already checked it and would have thrown an IllegalArgumentException), then you do not need to recheck; otherwise, you should double-check that the object is non-null before passing it along.

Since you do not check the parameters to your findElement and add functions, it is quite possible that the parameters are the culprits. Add the appropriate check and throw IllegalArgumentException if they are null. If, after you do that, you get an IllegalArgumentException, then you've solved your problem. If not, then you at least know that the problem is not the parameter and is elsewhere in the code.

Michael Aaron Safyan
+2  A: 

Post the entire class - this snippet is useless.

You're making two serious mistakes: failing to believe the compiler, and assuming that your code is correct.

If the JVM tells you that line 31 is the problem, believe it.

My guess is that setElements[i] is null.

duffymo
A: 

Hi,

Its working now, thanks to Lars,Igor and the rest who took time to critic the code, there's a logic error that wasn't check,anyway here's the corrected working code, lastly I'm bother am I doing cheating? :(

int findElement(String element) {
          int retval = 0;

            for ( int i = 0; i < setElements.length; i++) { //loop first to the array and only return -1 once we can't find it.         
       //setElements[i] != null is the NullPointerException killer :)


               if ( setElements[i] != null && setElements[i].equals(element) ) {
                  return retval = i;

               } 
            retval = -1; 
           }

          return retval;
       }

       void add(String newValue) {
            int elem = findElement(newValue);
            if( numberOfElements < maxNumberOfElements && elem == -1 ) { # == instead of != as I only need to add if elements is non-existing 
               setElements[numberOfElements] = newValue;
               numberOfElements++;
            } 
       }

with thanks, lupin

lupin
Don't forget to up-vote answers you found useful.
trashgod
By the way, if you used one of the java.util.List implementations instead of a String array, you could get completely rid of the findElement method and use List.indexOf(Object) instead. Of course, you may not be allowed to do that in your homework.
fish