1

I'm making this method remove() which takes a String word as argument, to delete from a global Array "words", but I keep getting a NullPointerException for some reason I cannot find, been stuck for hours.

Basically I check for if the word is in the first position, else if is in the last position, or else if it is in neither so I check all the array, and add the first half before the position of the word, and then add the second half after the position of the word in the array, as to skip it and "delete it". But I'm getting a NullPointerException in the for loop looking for the position of the word in the array. Code for the method is here:

public void remove(String a){

String[] temp_arr = new String[words.length-1]; // make array with 1 less length for deleted 

    if(words[0].equals(a)){ // if the word is the first in the array

        for(int x=0, z=1; x<temp_arr.length; x++,z++)
            temp_arr[x]=words[z];

        words = temp_arr;

    } else if(words[words.length-1].equals(a)){ // if the word is in the last position of the array

        for(int x=0, z=0; x<temp_arr.length; x++,z++)
            temp_arr[x] = words[z];

        words = temp_arr;

    } else{ // if the word is in neither first or last position of array

        // THIS IS WHERE the exception is thrown, in this for loop, in the if(words[k].equals(a))

        int k=0;
        for (; k<words.length; k++){ // find the position of the word to delete
            if (words[k].equals(a)) {
                break;
            }
        }

        for (int i = 0; i < k-1; i++){ // add first part of array before the word

            temp_arr[i] = words[i];
        }

        for(int c = k, b = k+1; c< temp_arr.length; c++,b++){
            temp_arr[c] = words[b];
        }

        words = temp_arr; // assign the new values to global array

    }
}

Also, if theres any suggestions for good coding practice would be appreciated, thanks!

** I can only use Arrays as my data structure for this method.

nullwriter
  • 785
  • 9
  • 34
  • On which line are you getting the exception? – Jeff Dec 14 '12 at 01:26
  • you could help us out by saying which line is throwing the exception. There seems to be precious little null checking. Assume the array is null until you have verfied that it is not. Assume all elements of the array are null before you have verified that they are not. – BevynQ Dec 14 '12 at 01:26
  • Is the `words` array fully populated? If there are any `null`s in the `words` array, it'll throw the exception when you try to call `.equals()`. If this isn't it, could you post the exception trace, and tell us the exact line that throws the exception. – wattostudios Dec 14 '12 at 01:28
  • edited to show where the exception is thrown – nullwriter Dec 14 '12 at 01:35
  • the array words is full, it is getting words from a random word generator – nullwriter Dec 14 '12 at 01:36
  • 1
    maybe you can try to perform some checks first? like `if (words != null)` then check `if (words.length() > 1)` also `if (a != null)`. For debugging, I think adding `try catch` then `printStackTrace` to see which line throws the exception. – Ghost_000_cs Dec 14 '12 at 01:38
  • This is an exercise in 0-based indexing and for loops. However, once you've worked through this, take a look at System.arrayCopy. It's much cleaner than for loops. – Devon_C_Miller Dec 14 '12 at 02:36

4 Answers4

2

Modify the condition like this

a.equals(words[0])

because you know the string value a. But dont know what value will come from array. So even null value comes from the array it does allow the null pointer exception.

MGPJ
  • 1,062
  • 1
  • 8
  • 15
  • Probably would want to do an `if(a == null) return;` at the top, but otherwise this looks quite likely. Which probably means there's a problem somewhere else in the code. – Jeff Dec 14 '12 at 01:40
  • Actually, this worked for me, changed all the conditions to a.equals(words[]) and it fixed it for me. Thanks a lot mate! – nullwriter Dec 14 '12 at 01:56
  • If `a` is `null` then you also get an exception. – Genzer Dec 14 '12 at 02:55
  • Definitely, but in this case, I'm sure a is not null. – nullwriter Dec 14 '12 at 11:04
1

I run your code and find a few errors, I correct somethings without changing the core idea: } else { // if the word is in neither first or last position of array

        // THIS IS WHERE the exception is thrown, in this for loop.

        int k = -1;
        for (int i = 0; i < words.length; i++) { // find the position of the word to delete
            if (words[i].equals(a)) {
                k=i;
                break;
            }
        }
        if(k<0)//if not exists
            return;

        for (int i = 0; i < k /*- 1*/; i++) { // add first part of array before the word

            temp_arr[i] = words[i];
        }

        for (int i = k; i < temp_arr.length; i++) {
            temp_arr[i] = words[i+1];
        }

        words = temp_arr; // assign the new values to global array

    }

If the original array could't have null elements I would do like this:

public static String[] remove(String words[] , String a) {
    int counter = 0;
    for (int i = 0; i < words.length; i++) {
        if( a.equals(words[i]) ){
            words[i] = null;
            counter++;
        }
    }
    if(counter==0){
        return words;
    }
    String[] words2 = new String[words.length - counter];
    int i=0;
    for (String string : words) {
        if(string!=null){
            words2[i++]=string;
        }
    }   
    return words2;
}
nms
  • 325
  • 1
  • 14
0

I would do that like this:

public void remove(String a) {
    List<String> tmp = new ArrayList<String>();

    for (String word : words) {
        if ((word != null) && (word.equals(a))) {
            continue;
        }

        tmp.add(word);
    }

    words = tmp.toArray(new String[]);
}
  • 1
    You could improve that by just `List – Jeff Dec 14 '12 at 01:34
  • I should add I can only use Arrays as my data structure – nullwriter Dec 14 '12 at 01:36
  • @Jeff `Arrays.asList()` returns a fixed-size list. We cant remove elements from that List. To solve that problem we can use `List tmp = new ArrayList(Arrays.asList(words))` – Pshemo Dec 14 '12 at 01:41
  • 1
    @Pshemo Oh, good call. So if you were going to use my version you'd have to throw it into some other collection first: `List tmp = new ArrayList(Arrays.asList(word));` or something - [sort of like this I suppose](http://stackoverflow.com/a/157950/781965) – Jeff Dec 14 '12 at 01:43
  • @Jeff Wow! Over 440 upvotes for such simple answer. Every day SO surprises me :D – Pshemo Dec 14 '12 at 01:50
0

I have a question for you:

Why oh why are you using an array? You should always use a collection (eg a List) unless you absolutely have to use an array (which is rare indeed).

If it were a List, you wouldn't even need this method, because List has the remove() method that does all this for you!

Bohemian
  • 412,405
  • 93
  • 575
  • 722
  • Given what it looks like the OP is doing, a `Set` might be *even more* appropriate – Jeff Dec 14 '12 at 01:38
  • This is a work I'm doing for university where I can only use Arrays as my data structure, surely I wouldnt use them if I was doing for my own use. But this is for problem solving purposes i guess – nullwriter Dec 14 '12 at 01:38