3
\$\begingroup\$

Description:

Below is one of the interview practice question. I think questions like these are to test the candidate's capability to write object oriented code. I being a candidate right now don't know what are the key points I should keep in point while white boarding questions like these. I have tried to write down my thought process as comments.

Goal:

The goal is to simulate a cost center.

import java.util.List;
import java.util.ArrayList;

/*

Problem statement

Design a call center such that:

1. There are three kinds of employees: Respondants, Managers and Directors
2. When a call comes then it is allocated to the Respondants first
3. If no Respondant is free or not able to handle it then its escalated to Managers.
4. If no Manager is free or not able to handle it then its escalated to Directors.

*/

/*
  My thought process

  Discovering core objects
  -------------------------
  Employee (Respondant, Manager, Director), Call, Caller, CallManager

  Discovering relationship
  -------------------------
  CallManager has many employees
  There can be only one call per Employee
  There can be only one caller per caller
  An Employee can manage only one caller at a time

  Go for most basic design i.e. CallManager managine everything
*/

public class Main {
  public static void main(String[] args) {
    System.out.println("Hello World");
    CallManager manager = new CallManager();

    manager.addRespondant(new Employee("R1", manager));
    manager.addRespondant(new Employee("R2", manager));
    manager.addRespondant(new Employee("R3", manager));

    manager.addManager(new Employee("M1", manager));
    manager.addManager(new Employee("M2", manager));
    manager.addManager(new Employee("M3", manager));

    manager.addDirector(new Employee("D1", manager));
    manager.addDirector(new Employee("D2", manager));

    Caller foo = new Caller("foo");

    manager.dispatch(foo); // should be R1
    manager.dispatch(new Call()); // should be R2
    manager.dispatch(new Call()); // should be R3
    manager.dispatch(new Call()); // should be M1
  }
}

class CallManager {
  List<List<Employee>> employeeLevels;
  List<List<Call>> waitQueue;

  CallManager() {
    this.employeeLevels = new ArrayList<>();
    this.employeeLevels.add(new ArrayList<>());
    this.employeeLevels.add(new ArrayList<>());
    this.employeeLevels.add(new ArrayList<>());
    this.waitQueue = new ArrayList<List<Call>>();
  }

  private Employee getCallHandler(Call call) {
    /* check all respondants */
    for (Employee respondant: employeeLevels.get(0)) {
      if (respondant.isFree()) return respondant;
    }

    /* check all managers */
    for (Employee manager: employeeLevels.get(1)) {
      if (manager.isFree()) return manager;
    }

    /* check all directors */
    for (Employee director: employeeLevels.get(2)) {
      if (director.isFree()) return director;
    }

    // No one is free
    return null; // returning null is not a good idea
  }

  public void addRespondant(Employee emp) {
    employeeLevels.get(0).add(emp);
  }

  public void addManager(Employee emp) {
    employeeLevels.get(1).add(emp);
  }

  public void addDirector(Employee emp) {
    employeeLevels.get(2).add(emp);
  }

  public void dispatch(Caller caller) {
    dispatch(new Call(caller));
  }

  public void dispatch(Call call) {
    /* check if any respondant is free */
    Employee handler = getCallHandler(call);
    if (handler == null) {
      System.out.println("Sorry, the line is busy, your call is going in wait queue");
      putCallInWaitQueue(call);
      return;
    }
    handler.assignCall(call);
    call.setEmployee(handler);
  }

  public void putCallInWaitQueue(Call call) {
    waitQueue.get(call.getRank()).add(call);
  }
}


class Employee {
  private String name;
  private Call currentCall;
  private CallManager callManager;

  Employee(String name, CallManager callManager) {
    this.name = name;
    this.callManager = callManager; // this is the required depedency hence must be there in constructor
  }

  public boolean isFree() {
    return this.currentCall == null;
  }

  private void escalateCall() {
    if (!isFree()) {
      currentCall.incrementRank();
      callManager.putCallInWaitQueue(currentCall);
    }
  }

  public void assignCall(Call call) {
    System.out.println(name + " Received call!");
    currentCall = call;
  }
}

final class Call { // final by default
  private int rank;
  private Caller caller;
  private Employee employee;

  Call() {
    this.rank = 0;
  }

  Call(Caller caller) {
    super();
    this.caller = caller;
  }

  public int getRank() {
    return rank;
  }

  public void incrementRank() {
    this.rank += 1;
  }

  public void setCaller(Caller caller) {
    this.caller = caller;
  }

  public void setEmployee(Employee employee) {
    this.employee = employee;
  }
}

final class Caller {
  private String name;

  Caller(String name) {
    this.name = name;
  }
}

Questions:

Apart from comment on current design, I would like to get an advice on how to find the classes to model and their behaviour?

\$\endgroup\$
  • 2
    \$\begingroup\$ the quiz want ya to use the chain responsibility pattern :) \$\endgroup\$ – wBacz Sep 6 '17 at 18:04
5
\$\begingroup\$

Thanks for sharing your code, it's looking good but I think we can make some improvements.

Currently, the getCallHandler(Call call) method doesn't actually make use of call! We can just remove that, so we end up with getCallHandler(). And then when we call it later, we don't need to pass in a Call object.

you say that returning null is a bad idea. I agree with you, and would suggestion you use an Optional instead. This was introduced in Java 8. It's intended purpose is to act as a value to return when the actual return value might not be there. To me, your example looks like a good use case!

Let's see what your method looks like if we use an Optional.

private Optional<Employee> getCallHandler() {
    /* check all respondants */
        for (Employee respondant: employeeLevels.get(0)) {
            if (respondant.isFree()) return Optional.of(respondant);
        }

    /* check all managers */
        for (Employee manager: employeeLevels.get(1)) {
            if (manager.isFree()) return Optional.of(manager);
        }

    /* check all directors */
        for (Employee director: employeeLevels.get(2)) {
            if (director.isFree()) return Optional.of(director);
        }

        // No one is free
        return Optional.empty(); // returning null is not a good idea - I agree :)
    }

However this does mean we now need to change the calling code.

public void dispatch(Call call) {
/* check if any respondant is free */
    Optional<Employee> handler = getCallHandler();
    if (!handler.isPresent()) {
        System.out.println("Sorry, the line is busy, your call is going in wait queue");
        putCallInWaitQueue(call);
        return;
    }
    handler.get().assignCall(call);
    call.setEmployee(handler.get());
}

Now we don't need to worry about passing null around to indicate "no value".

  1. There are three kinds of employees: Respondants, Managers and Directors

To me this is someone screaming that they want you to use inheritance in some form or another.

In your current code, you have a single Employee class, and you maintain the different levels of Employee in 3 different lists, and then you keep track of them by index, 0, 1 and 2. I think here we can make Employee an abstract base class, and create 3 concrete implementations. These will be Respondant, Manager and Director.

These could look like

class Respondant extends Employee {

    Respondant(String name, CallManager callManager) {
        super(name, callManager);
        this.priority = 1;
    }
}

class Manager extends Employee {

    Manager(String name, CallManager callManager) {
        super(name, callManager);
        this.priority = 2;
    }
}

class Director extends Employee {

    Director(String name, CallManager callManager) {
        super(name, callManager);
        this.priority = 3;
    }
}

These are all very similar, the only thing that's different here is the priority. I've set these to match the question description. At the moment they're all very bare boned, but you could go ahead and add different ways that they might respond to the calls. Even just different print statements.

Now that we have these 3 subclasses, we don't need 3 lists, we need 1. Let's update this.

 CallManager() {
        this.employees = new ArrayList<>();
        this.waitQueue = new ArrayList<>();
    }

now we just put all our Employees there.

At the moment we have 3 different methods for adding employees, again we only need 1.

public void addEmployee(Employee emp) {
    employees.add(emp);
}

now it's all well and good that we have 3 subclasses, but we're going to need to be able to compare them, we need to add a way for an Employee to give it's priority, so let's just use a getter. (we don't need a setter)

class Employee {
    ...
    protected int priority;
    ...
    public int getPriority() {
        return priority;
    }
}

okay so now we have 3 subclasses and can get their priority, so all we have to do now is sort our list, and then remove all the ones that aren't free. Then, at position 0 we will always have the Employee at highest priority!

private Optional<Employee> getCallHandler() {
        List<Employee> activeEmployees = employees.stream()
                .filter(Employee::isFree)
                .sorted(Comparator.comparingInt(Employee::getPriority))
                .collect(Collectors.toList());

        if(!activeEmployees.isEmpty()){
            return Optional.of(activeEmployees.get(0)); // the sorted list, putting the top priority at position 0.
        }
        return Optional.empty(); // returning null is not a good idea
    }

If you're not used to Java streams this change may look a bit complicated, but really all we're doing here is getting the Stream of the list of employes, filtering out based on the isFree method, and sorting by the getPriority value.

So now we've deleted some unneeded methods in the form of addX addY. We're now just handling a single list, not a list of 3 lists. It's open to modification in that we can add any other Employee type in the future and this code will still work!

With this new code, this is what your main method can look like.

public class Main {
    public static void main(String[] args) {
        System.out.println("Hello World");
        CallManager manager = new CallManager();

        manager.addEmployee(new Respondant("R1", manager));
        manager.addEmployee(new Respondant("R2", manager));
        manager.addEmployee(new Respondant("R3", manager));

        manager.addEmployee(new Manager("M1", manager));
        manager.addEmployee(new Manager("M2", manager));
        manager.addEmployee(new Manager("M3", manager));

        manager.addEmployee(new Director("D1", manager));
        manager.addEmployee(new Director("D2", manager));

        Caller foo = new Caller("foo");

        manager.dispatch(foo); // should be R1
        manager.dispatch(new Call()); // should be R2
        manager.dispatch(new Call()); // should be R3
        manager.dispatch(new Call()); // should be M1
    }
}

And the output is

Hello World
R1 Received call!
R2 Received call!
R3 Received call!
M1 Received call!

Hopefully this review was helpful for you!

\$\endgroup\$
  • \$\begingroup\$ I don't know java, but I was able to understand every word you said, nice job! +1 \$\endgroup\$ – JAD Sep 7 '17 at 17:38
  • \$\begingroup\$ @chatton thanks for the detailed feedback but to not disappoint you, you just scratched the surface here, my main concern is defining proper objects, relationship between them etc. Also, you traded off space with time in getting the idle employee, but yes that is a trade off. Overall good advices but I wanted more :) \$\endgroup\$ – CodeYogi Sep 7 '17 at 19:19
2
\$\begingroup\$
 List<Employee> activeEmployees = employees.stream()
                .filter(Employee::isFree)
                .sorted(Comparator.comparingInt(Employee::getPriority))
                .collect(Collectors.toList());

why cannot use findAny with parallel stream?, rather collecting List?

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.