0

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.

Evgeniy
  • 140
  • 1
  • 8
  • 2
    Does this answer your question? [What is a NullPointerException, and how do I fix it?](https://stackoverflow.com/questions/218384/what-is-a-nullpointerexception-and-how-do-i-fix-it) – David Nov 22 '22 at 16:17
  • 3
    That's the most complicated code I've ever seen to maintain and delete from a simple list of objects. Why not store the employees in a `List` and avoid all of the headaches of rebuilding an array? I don't understand the complexity of using streams for this operation either. You end up iterating over the entire array after building a separate list via a stream. I don't get what that buys you. – CryptoFool Nov 22 '22 at 16:20
  • 2
    Why do you keep employees as array instead of `List`? It's more appropriate data structure for your use case, and you are already using a list in your `removeEmployee` method. – Chaosfire Nov 22 '22 at 16:21
  • 1
    Is returning the deleted `Employee` object absolutely required? Or is it enough to know that an employee was deleted or not? – Basil Bourque Nov 22 '22 at 16:24
  • It's a homework task. And according to the condition of the task, an array should be used, not a List. – Evgeniy Nov 22 '22 at 16:27
  • Yes, returning the deleted Employee object absolutely required according to the task. Namely Employee, not just boolean. – Evgeniy Nov 22 '22 at 16:29
  • So, what *should* happen if there is an attempt to delete an Employee who doesn't exist? – Old Dog Programmer Nov 22 '22 at 16:30
  • 2
    If it's homework i don't think you are supposed to use streams either. Your assignment probably requires you to learn how to resize the array, shift its' elements, keep count of current elements and so on. At the very least you should try to implement it this way. – Chaosfire Nov 22 '22 at 16:31
  • 4
    Usually if a homework tells you to use `array`s it means *just* `array`s - the idea being that you have to learn to do the nitty gritty yourself. So, you still shouldn't be mixing in all the more advanced features like streams. – Edward Peters Nov 22 '22 at 16:31

3 Answers3

2

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;

}
  • 1
    Please check [How do I ask and answer homework questions?](https://meta.stackoverflow.com/questions/334822/how-do-i-ask-and-answer-homework-questions). Directly providing a solution to a student is totally not in his best interest. – Chaosfire Nov 22 '22 at 16:36
  • you are correct. However, since only the last function qualifies as a valid solution to the task, I intentionally did not provide a complete solution. I think that completing the parameters to the arraycopy function requires understanding how the solution works ;) – Lorenz Hetterich Nov 22 '22 at 16:41
  • Yes, but the stream based solution works and can be used as is. Using it will most probably fail his assignment, but it in the end he would have learned nothing. And we all know that having a ready solution can be quite the temptation for any student :) – Chaosfire Nov 22 '22 at 17:08
  • Actually, I think even the last solution would fail the assignment. While question states "the length ... must be reduced", I think the actual requirement is to reduce the size and keep an array with size non-null elements followed by capacity-size null elements. (At least this would make more sense to me) – Lorenz Hetterich Nov 22 '22 at 17:17
1

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;
}
Alexander Ivanchenko
  • 25,667
  • 5
  • 22
  • 46
0

Considering it's homework and constraints mentioned, i believe you are supposed to do all the work using the array only.

I'll provide some guideline and leave the actual implementation to you:

public class Company {

  private Employee[] employees;
  private int size;

  public Employee removeEmployee(int id) {
    int index = -1;
    //find the index of employee with required id, you have mostly done that
    if (index == -1) {
      return null;
    }
    //save found employee to variable
    //remove from array
    //shift array to the left

    //do not forget to use and reassign size variable where appropriate
  }

  //some extra
  public void addEmployee(Employee employee) {
    //resize array if necessary
    //add employee at correct position in array

    //do not forget to use and reassign size variable where appropriate
  }
}

If you get stuck, you can look at the ArrayList class, your task is basically a simplified version of it. I strongly advise you to use this as source of inspiration only and not to copy the source code!!!

Chaosfire
  • 4,818
  • 4
  • 8
  • 23