How to save in an array with a for loop with check of existing values in Java

Question

I'm a newbie in Java, but i'm taking a course where we write a web-app which is able to save, store, edit and show resumes. I'm stuck with the 'save' method, where we send a resume as parameter and have to check by resume's UUID:

• if the UUID already exists in the array - an error message will be shown,

• if the UUID is not in the array - the resume will be saved.

I've made my method with a temp boolean, but according a task it's not right, so i'm trying to solve it with a for loop and an if statement with break, but i can't understand how to save the resume because NPE throw in if statement. Here is my code:

    public class ArrayStorage {
    private Resume[] storage = new Resume[10000];
    private int size;

    public void save(Resume resume) {
        for (int i = 0; i <= size; i++) {
            if (storage[i].getUuid().equals(r.getUuid())) {
                System.out.println("ERROR: Resume with " + storage[i].getUuid() + " is already exist!");
                break;
            }
            storage[size] = resume;
            size++;
        }
    }

Size variable is there to avoid checking the whole array, just elements that were stored.

Edited. I forgot an equal sign in a for loop, and this is an error

Exception in thread "main" java.lang.NullPointerException
at com.webapp.storage.ArrayStorage.save(ArrayStorage.java:23)
at com.webapp.MainTestArrayStorage.main(MainTestArrayStorage.java:22)

In MainTestArrayStorage i have those fields

 Resume r1 = new Resume();
    r1.setUuid("uuid1");
    Resume r2 = new Resume();
    r2.setUuid("uuid2");
    Resume r3 = new Resume();
    r3.setUuid("uuid3");
    Resume r4 = new Resume();
    r4.setUuid("uuid1");

    ARRAY_STORAGE.save(r1);
    ARRAY_STORAGE.save(r2);
    ARRAY_STORAGE.save(r3);
    ARRAY_STORAGE.save(r4);

The last line is for check method, that shouldn't save it, because it has the same UUID If you need any additional information i'll be glad to answer. Thanks.

P.S. Sorry for my English, i'm not good at it.

Answer 1

I think your are confusing size and the end condition of the for loop here. The way it works in your example is that it will go through the array until it finds or duplicate or until a NPE is thrown because storage[i] is null. But because you increase both the index (i) and the stop condition (size) at the same time, you never stop. You will break when you find a duplicate, get a NPE when you find an empty slot, or an IndexArrayOutOfBoundsException when you reach the end of the full storage.

In cases like this, it is helpful to first get a very good idea of what you need to do before writing any code. If you cannot describe the method in normal words, you will have a hard time writing it.

Try this:

• Iterate through the whole storage
• if you find an empty slot, put the new resume into the storage
• stop when you find a duplicate, show an error, and abort

Now write it down step by step:

public void save(Resume resume) {
    for (int i = 0; i < storage.length; i++) {
        if (storage[i] == null) {
            // good result: found an empty slot
            storage[i] = resume;
            size = i + 1;
            return;
        }
        if (storage[i].getUuid().equals(resume.getUuid())) {
            // bad result: found a duplicate
            System.out.println("ERROR: Resume with " + storage[i].getUuid() + " is already exist!");
            return;
        }
    }

    // bad result: storage full
    System.out.println("ERROR: Storage full");
}

Answer 2

You didn't tell us where you get the NPE so let's analyze your code (you should debug though):

public void save(Resume resume) {
    //size is initialized to 0, so all is fine here
    for (int i = 0; i < size; i++) {
        //now here's the candidate for the NPE
        if (storage[i].getUuid().equals(resume.getUuid())) {
            System.out.println("ERROR: Resume with " + storage[i].getUuid() + " is already exist!");
            break;
        }
        //this could cause the NPE in a later iteration
        storage[size] = resume;
        size++;
    }
}

As you can see the only part where an NPE should happen (assuming no concurrent access to that class) is storage[i].getUuid().equals(resume.getUuid()).

What could be null here?

1. storage[i] might be null if when calling storage[size] = resume; resume was null
2. storage[i].getUuid() might return null
3. resume could be null
4. resume.getUuid() could return null but that should not cause the NPE immediately

As you can see, the first 3 options might cause the NPE immediately and there could be 2 reasons for this:

1. you pass a null resume
2. the resume you pass has no UUID

The first reason might cause the NPE right away, due to the call resume.getUuid(). It also might cause the NPE in the second call because then storage[size] = resume; would be equivalent to storage[0] = null; (all but the first call should see size > 0).

The second reason would cause the next call to get the NPE because then you'd call equals() on a null UUID.

To fix that, and assuming a resume must always have a UUID, I'd add the following at the top of the save(Resume resume) method:

public void save(Resume resume) {
  if( resume == null || resume.getUuid() == null ) {
    //Log the error here
    return;
  }
  //the rest of your code goes here
}

UPDATE:

I just saw you changed your loop condition to i <= size which probably causes the error (and which your original question didn't contain): if size is 0 (no elements yet), you'll still execute the loop body once (because i == 0) and hence call storage[0].getUuid(), which is bound to produce an NPE because all elements in storage are still null. Change the condition to i < size and it should be ok.

Answer 3

Prior of inserting, check first if the storage is empty and initialize it with at least one element. Also verify that the storage is not full before trying to add new element.

Try this out :

public class ArrayStorage {
    private Resume[] storage = new Resume[10000];
    private int size;

    public void save(Resume resume) {
        if(size == 0){
            storage[size] = resume;
            size++;
        } else {
            for (int i = 0; i < size; i++) {
                if(size == storage.length){
                    System.out.println("ERROR: Storage is full");
                    break;
                }
                if (storage[i].getUuid().equals(r.getUuid())) {
                    System.out.println("ERROR: Resume with " + storage[i].getUuid() + " is already exist!");
                    break;
                }
                storage[size] = resume;
                size++;
            }
        }
    }
}

Answer 4

Without the actual error message I assume that you ran into a NPE due to resume.getUuid() returning null (maybe a resume is also null to test you). I also assume that resumes in storage are valid (they have a Uuid).

public class ArrayStorage {
private Resume[] storage = new Resume[10000];
private int size;

public void save(Resume resume) {
    for (int i = 0; i < size; i++) {
        if (resumse != null 
                 && resume.getUuid() != null 
                 && storage[i].getUuid().equals(resume.getUuid())) {
            System.out.println("ERROR: Resume with " + storage[i].getUuid() + " is already exist!");
            break;
        }
        storage[size] = resume;
        size++;
    }
}

Answer 5

You should use a List instead of an Array. Lists have variable size and you dont have to create ten thousand null objects. You also wouldnt have to check for the size, just iterate over all objects in the List and compare the UUIDs. Of course only if you dont HAVE to use Arrays.

Answer 6

Why do not you consider Set instead array? You could implement UUID-based equals and hashCode in your Resume class, then simple use HashMap.putIfAbsent to store your data.

Another approach (jdk 8+) is to use Stream.anyMath:

if (!Stream.of(storage).parallel().anyMatch(v -> v.getUuid().equals(r.getUuid())) { storage[size++] = resume; }