2
\$\begingroup\$

I wanted to give a shot to Modelling a Call Center from Cracking the Coding Interview. The problem statement is as follows:

You have a call center with three levels of employees: Respondent, Manager, Director. An incoming phone call must be allocated to a respondent who is free. If the respondent cannot handle the call, they must escalate it to a manager. If the manager is not free or not able to handle it, the call should be escalated to a director.

Here is my implementation in Java, which accepts calls from the command line in the form of: level,duration. For example entering: 0,25 represents a call that can be handled by a Respondant (level 0) and that will take 25 seconds to complete.

The code should be easy to just copy / paste to an IDE or just compile with javac in case you want to play with it in your local environment. Simply save it in CallCenter.java in global package and you are good to go.

import java.util.Arrays;
import java.util.List;
import java.util.Scanner;
import java.util.concurrent.ArrayBlockingQueue;
import java.util.concurrent.BlockingQueue;

import static java.lang.Integer.parseInt;

public class CallCenter {

    static BlockingQueue<Call> respondantQueue = new ArrayBlockingQueue<>(20);
    static BlockingQueue<Call> directorQueue = new ArrayBlockingQueue<>(20);

    static List<Manager> managers = Arrays.asList(new Manager("Manager - 1"), new Manager("Manager - 2"));

    public static void main(String[] args) throws Exception {
        new CallCenter().operate();
    }

    void operate() throws InterruptedException {
        Arrays.asList(new Respondant("Respondant - 1"), new Respondant("Respondant - 2"));
        Arrays.asList(new Director("The Director"));

        System.out.println("Enter values representing call in the format: 0,25.");
        System.out.println("First value is level required, second value is call duration.");
        Scanner scanner = new Scanner(System.in);
        while (true) {
            String s = scanner.nextLine();
            if ("q".equals(s)) {
                break;
            }
            String[] split = s.split(",");
            Call call = new Call(parseInt(split[0]), parseInt(split[1]));
            dispatchCall(call);
        }
        scanner.close();
        System.exit(0);
    }

    private void dispatchCall(Call call) throws InterruptedException {
        respondantQueue.put(call);
    }
}

class Call {

    static int idCounter = 0;

    int id;
    int durationInSeconds;
    int level;

    public Call(int level, int durationInSeconds) {
        this.level = level;
        this.durationInSeconds = durationInSeconds;
        this.id = idCounter++;
    }

    @Override
    public String toString() {
        return "Call{" + "id=" + id + ", duration(sec)=" + durationInSeconds + ", level=" + level + '}';
    }
}

class Respondant {
    String name;

    public Respondant(String name) {
        this.name = name;
        new Thread(() -> {
            while (true) {
                try {
                    Call call = CallCenter.respondantQueue.take();
                    if (call.level == 0) {
                        System.out.println(name + " handling call: " + call);
                        Thread.sleep(call.durationInSeconds * 1000);
                    } else {
                        boolean managerHandled = false;
                        for (Manager manager : CallCenter.managers) {
                            if (!manager.isBusy) {
                                manager.acceptCall(call);
                                managerHandled = true;
                                break;
                            }
                        }
                        if (!managerHandled) {
                            CallCenter.directorQueue.put(call);
                        }
                    }
                } catch (InterruptedException ignored) {}
            }
        }).start();
    }
}

class Manager {
    String name;
    volatile boolean isBusy = false;

    public Manager(String name) {
        this.name = name;
    }

    void acceptCall(Call call) {
        new Thread(() -> {
            try {
                isBusy = true;
                if (call.level == 1) {
                    System.out.println(name + " handling call: " + call);
                    Thread.sleep(call.durationInSeconds * 1000);
                } else {
                    CallCenter.directorQueue.put(call);
                }
                isBusy = false;
            } catch (InterruptedException ignored) {}
        }).start();
    }
}

class Director {
    String name;

    public Director(String name) {
        this.name = name;
        new Thread(() -> {
            while (true) {
                try {
                    Call call = CallCenter.directorQueue.take();
                    System.out.println(name + " handling call: " + call);
                    Thread.sleep(call.durationInSeconds * 1000 / 4); // Director handles calls 4 times faster!
                } catch (InterruptedException ignored) {}
            }
        }).start();
    }
}

A sample run for me is as follows:

Enter values representing call in the format: 0,25.
First value is level required, second value is call duration.
0,5
Respondant - 1 handling call: Call{id=0, duration(sec)=5, level=0}
0,5
Respondant - 2 handling call: Call{id=1, duration(sec)=5, level=0}
1,5
Manager - 1 handling call: Call{id=2, duration(sec)=5, level=1}
1,5
Manager - 2 handling call: Call{id=3, duration(sec)=5, level=1}
1,5
The Director handling call: Call{id=4, duration(sec)=5, level=1}
\$\endgroup\$
  • \$\begingroup\$ I won't do a proper review, but it seems to make more sense to use a thread pool than start a new thread every time. It also doesn't make much sense for the thread to be created in the different classes, so a thread pool would solve that. I'd probably also create an interface with acceptCall/canHandle methods as well. \$\endgroup\$ – Chris Schneider Sep 16 at 3:37
  • \$\begingroup\$ @ChrisSchneider I am creating a new thread for every instance of Respondent and Director which is required and cannot be replaced by a thread pool I think. For Manager class, hmm maybe. Thank you for your input. \$\endgroup\$ – Koray Tugay Sep 16 at 10:39
1
\$\begingroup\$

It's hard to give you a full review, because the code complexity is too high..

Little Typo

Respondent instead of Respondant.

You did not Implement the Task

If the respondent cannot handle the call, they must escalate it to a manager.

The Respondent handles his call:

if (call.level == 0) {
 System.out.println(name + " handling call: " + call);
 Thread.sleep(call.durationInSeconds * 1000);
}

When the call is of level 0 it is a call which gets handles by an Respondent. But where is the escalate part?

Currently the call has an fixed level which means that the caller can choose if he wants to talk with an Respondent or an Manager. Instead the caller should only allowed to call a Respondent and the Respondent has to delegate the call to a Manager.

main in its own Class

I would create a new class for the main which is currently in CallCenter. This would reduce the complexity and makes a step farther to make the components more reusable.

Encapsulation

From Wikipedia:

Encapsulation, in object-oriented programming, is the bundling of data with the methods that operate on that data, or the restricting of direct access to some of an objects components. Encapsulation is used to hide the values or state of a structured data object inside a class, preventing unauthorized parties' direct access to them.

On closer inspection, all fields have no access modifier, which means that they are all [package-private(https://docs.oracle.com/javase/tutorial/java/java/accesscontrol.html) at the moment so other classes can access them directly.

As an example we could look into Call:

class Call {
   /* ... */

   int id;
   int durationInSeconds;
   int level;

   /* ... */
}

Now lets focus on durationInSeconds which gets used directly by Respondent, Manager and Director:

Thread.sleep(call.durationInSeconds * 1000);

The goal would be to make all fields private, like:

class Call {
    /* ... */

    private int id;
    private int durationInSeconds;
    private int level;

    /* ... */
}

Static Variables

public class CallCenter {

 static BlockingQueue<Call> respondantQueue = new ArrayBlockingQueue<>(20);
 static BlockingQueue<Call> directorQueue = new ArrayBlockingQueue<>(20);

 /* ... */
}

Imagine you want to open a new call center. With the declaration static the old and the new call center divide the queues and with the current implementation the call can not be differentiated between old and new call center: The new call center could handle calls from the old call center..

Useless Variables

To identify a Call it has an id, but the id never gets accessed and is only in the Call#toString. After we delete id we will see that idCounter is useless too..

If the id would not be useless it would be good to use it in an equal-Method and move the idCounter to a component which creates the Call - for example the CallCenter or a new class like a CallFactory.

Business Logic in Constructor

Arrays.asList(new Director("The Director"));

Without knowing the code, I would not expect, that the line above executes the business logic of the director.

Additionally the constructor becomes a high level of complexity. When we look into Respondants constructor: it has 6 levels of indentation!

Some OOP

I think a suitable design pattern would be the Observer Design Pattern.

Every time the CallCenter receives a Call it can inform the employees.

\$\endgroup\$
  • \$\begingroup\$ When the call is of level 0 it is a call which gets handles by an Respondent. But where is the escalate part? -> It is when the call is not level 0. \$\endgroup\$ – Koray Tugay Oct 4 at 22:41
  • \$\begingroup\$ I added a paragraph to make it clearer. In short: The Respondent never delegates the call. The Manager only receives a call, when the caller calls with the level 1. But actually the caller should always call with level 0. \$\endgroup\$ – Roman Oct 5 at 7:42
  • \$\begingroup\$ Chain of responsibility is the design pattern here. Not observer \$\endgroup\$ – Gilad Oct 5 at 9:00
  • \$\begingroup\$ codereview.stackexchange.com/questions/202690/… \$\endgroup\$ – Gilad Oct 5 at 9:01
  • \$\begingroup\$ @Roman But actually the caller should always call with level 0. -> No. \$\endgroup\$ – Koray Tugay Oct 5 at 11:50

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.