Given the following task. We have an Employee and a Company classes. Each instance of Employee class is stored in array Employee[] employees in the Company class. I need a method which removes an instance of Employee in the array Employee[] employees by id.

I managed to write the following code:

public class Employee {
    protected final int id;
    protected String name;

    public Employee(int id, String name) {
        this.id = id;
        this.name= name;
    }
    public int getId() {
        return id;
    }
}

public class Company {
    private Employee[] employees;
    private int size;
    private static final int defaultCapacity = 5;
    
    public Company() {
        this(defaultCapacity);
    }
    
    public Company(int capacity) {
        if (capacity <= 0)
             throw new RuntimeException("capacity is required");
        employees = new Employee[capacity];
    }

    public Employee removeEmployee(int id) {
        Collection<Employee> employeeList = Arrays.asList(employees)
                                                  .stream()
                                                  .filter(Objects::nonNull)
                                                  .collect(Collectors.toList());
        
        Employee[] employeeArray = employeeList.toArray(Employee[]::new);
        for (int i = 0; i < size; i  ) {
            if(employeeArray[i].getId() == id) {
                Employee removedEmployee = employees[i];
                employeeList.remove(employeeArray[i]);
                employees = employeeList
                            .stream()
                            .filter(Objects::nonNull)
                            .toArray(Employee[]::new);
                return removedEmployee;
            }
        }
        return null;
    }

}

The problem is that my method public Employee removeEmployee(int id) throws NullPointerException if an element for removal is not found.

Question:

1. How can I rewrite the method public Employee removeEmployee(int id) using, for instance, Streams API and Optional in oder to get rid of NullPointerException in the method public Employee removeEmployee(int id)?

N.B.: The length of the array Employee[] employees declared in the class Company must be reduced after the element has been successfully removed.

CodePudding user response：

There is a lot of ways to get rid of the NullPointerException here. If you want to keep using the stream API, you may want to use filter and findAny. For example, you could modify the method to the following:

public Employee removeEmployee(int id) {
    Optional<Employee> employee = Arrays.stream(employees)
        .filter(Objects::nonNull)
        .filter(x -> x.getId() == id).
        .findAny();
    if(employee.isEmpty())
        return null;
    employees = Arrays.stream(employees).filter(x -> x != employee.get()).toArray(Employee[]::new);
    return employee.get();
}

However, I would highly advise using a List or even a Map instead of an Array for employees as this makes things way easier and faster:

public Employee removeEmployee(int id){
    Optional<Employee> toRemove = employees.stream().filter(x -> x.getId() == id).findAny();
    if(toRemove.isEmpty())
        return null;
    employees.remove(toRemove.get());
    return toRemove.get();

}

Or not to use the Stream API:

public Employee removeEmployee(int id){
    int idx;
    for(idx = 0; idx < employees.length; idx  ){
        if(employees[idx] != null && employees[idx].getId() == id)
            break;
    }
    if(idx == employees.length)
        return null;

    Employee value = employees[idx];
    
    Employee[] newArr = new Employee[employees.length - 1];

    // the parameters here are left as an exercise to the reader :P
    System.arraycopy(newArr, ...);
    System.arraycopy(newArr, ...);

    employees = newArr;

    return value;

}

CodePudding user response：

The length of the array Employee[] employees declared in the class Company must be reduced after the element has been successfully removed.

Streams doesn't buy you a lot in this case.

What you're supposed to do is to find the element with the target id, and if such an element exists, allocate a new array in memory with a length smaller by 1 copy all the elements apart from the one that was found, and assign employees with the reference to the new array.

To reduce the length, we can make use of the System.arraycopy(). First copy the elements before the target, and then after the target.

That's how it would look like with a plain index-based for-loop.

public Employee removeEmployee(int id) {
    Employee result = null;
    int index = -1;
    
    for (int i = 0; i < employees.length; i  ) {
        if (employees[i] != null && employees[i].getId() == id) {
            result = employees[i];
            employees[i] = null;
            break;
        }
    }
    if (result != null) {
        reduceLength(index);
    }
    return result;
}

public void reduceLength(int i) {
    Employee[] newEmployees = new Employee[employees.length - 1];
    System.arraycopy(employees, 0, newEmployees, 0, i);
    System.arraycopy(employees, i   1, newEmployees, i, employees.length - (i   1));
    employees = newEmployees;
}

If you want to do weird stuff and use Stream API and Optional at all costs, here how it can be done (but I would recommend to stick with the code above):

public Optional<Employee> removeEmployee(int id) {
    Optional<Integer> index = IntStream.range(0, employees.length)
        .filter(i -> employees[i] != null)
        .filter(i -> employees[i].getId() == id)
        .boxed()      // otherwise will get OptionalInt which lacks map() method
        .findFirst();
    
    Optional<Employee> result = index.map(i -> employees[i]);
    index.ifPresent(this::reduceLength);
    
    return result;
}