# Modelling a Call Center

This is the requirement I have (from the book: Cracking the Coding Interview)

Imagine you have a call center with three levels of employees: fresher, technical lead (TL), and product manager (PM). There can be multiple employees, but only one TL or PM. An incoming telephone call must be allocated to a fresher who is free. If a fresher can't handle the call, he or she must escalate the call to technical lead. If the TL is not free or not able to handle it, then the call should be escalated to PM. Design the classes and data structures for this problem. Implement a method getCallHandler().

And this is my implementation:

public interface CallAllocator {
public Employee getCallHandler() throws NoEmployeeInTheHouseException;
void setPM(ProductManager productManager);
}


Implementation for the interface:

public class CallAllocatorImpl implements CallAllocator {

private ProductManager productManager;
private List<Fresher> freshers = new ArrayList<Fresher>();

@Override
public Employee getCallHandler() throws NoEmployeeInTheHouseException {

if (freshers.isEmpty() && technicalLead == null && productManager == null) {
throw new NoEmployeeInTheHouseException();
}

if (!freshers.isEmpty()) {
Employee fresher = freshers.get(new Random().nextInt(freshers.size()));
if (fresher.getCanHandle()) {
return fresher;
}
}

}

if (productManager != null && productManager.getCanHandle()) {
return productManager;
}

throw new NoEmployeeInTheHouseException();

}
@Override
}
@Override
public void setPM(ProductManager productManager) {
this.productManager = productManager;
}
@Override
if (fresher.isFree()) {
}
}
}


Employee class:

public class Employee {
private boolean free;
private boolean canHandle;
public boolean isFree() {
return free;
}
public void setFree(boolean free) {
this.free = free;
}
public boolean getCanHandle() {
return canHandle;
}
public void setCanHandle(boolean canHandle) {
this.canHandle = canHandle;
}
}


I have 3 classes with names: Fresher, TechnicalLead and ProductManager. They all extend Employee but do not override any methods or anything..

And this is my TestClass:

public class TestClass {

public static void main(String[] args) throws NoEmployeeInTheHouseException {

CallAllocator callAllocator = new CallAllocatorImpl();

Fresher fresherOne = new Fresher();
fresherOne.setCanHandle(false);
fresherOne.setFree(true);

Fresher fresherTwo = new Fresher();
fresherTwo.setCanHandle(true);
fresherTwo.setFree(true);

Fresher fresherThree = new Fresher();
fresherThree.setCanHandle(false);
fresherThree.setFree(true);

Fresher fresherFour = new Fresher();
fresherFour.setCanHandle(false);
fresherFour.setFree(false);

ProductManager productManager = new ProductManager();
productManager.setCanHandle(true);
productManager.setFree(true);

callAllocator.setPM(productManager);

Employee callHandler = callAllocator.getCallHandler();
System.out.println(callHandler.getClass().getSimpleName());
}
}


So how can I improve this code? Any suggestions?

-

Firstly, I'm not sure you're actually implementing the requirement as it's written. The description says:

An incoming call must be allocated to a fresher who is free. If a fresher can't handle the call, he or she must escalate to technical lead.

This sounds like if there are no free freshers, the call shouldn't be handled at all (an exception thrown?), rather than skipping to the TL. This sort of makes sense as a real world requirement: it may be preferable for a caller to have to call back a bit later if there are no freshers available, rather than to waste the technical lead's time with a call that a fresher should be able to handle. Whatever calls this could, for example, plan to catch that exception and do addCallToUnhandledCallQueue() or whatever.

This also goes a bit more to the meat of the question, which I believe is trying to get you to answer with the chain-of-responsibility pattern. In this pattern each object (in this case an Employee) responsible for processing a command (in this case a call) contains logic to check whether it is capable of processing a given command, and if not, also knows the next object in the chain to call.

One benefit from this pattern is adherence to the open/close principle. As you'll see below, doing something like adding a new employee type or changing the structure a bit is unlikely to require you to fiddle around with if{...} else{...} logic in getCallHandler(). Additionally, it means that Employees only need to know about their immediate boss, rather than some master class having to know and persist the entire employee structure (which would fast become unpleasant, especially if you need to add other methods which also require knowing this structure).

A meta-benefit, given that this is apparently to be approached as an interview question, is that if somebody asked me this in an interview, I'd be pretty sure they'd want me to talk about this pattern, so even if for whatever reason you ultimately decide there's a better solution, it's important to understand this one if only to be able to describe intelligently why you reject it.

So using this pattern, your getCallHandler(Call call) method would look something like this:

Employee fresher = getAnyFreeFresher(); //Should throw if there are none
return fresher.handle(call);


Then the Employee class would look something like:

public class Employee{
private Employee boss;

private bool canHandle(Call call){
//...
}

public Employee handle(Call call){
if(canHandle(call)){
return this;
}
if(boss == null){
//Nobody in the chain could handle, throw
}
return boss.handle(call);
}
}


That's a very rough outline, there's detail to fill in on how bosses are set, and you'd probably want employee types to inherit from Employee to implement canHandle, etc.

-
Thanks for the answer, I see your point. Maybe you are right, maybe I totally got the requirement wrong. :) –  Koray Tugay Feb 28 at 19:32
Absolute agreement on the main point: you are not solving a problem, you are in an interview. Your priority is not to understand the problem; it is to understand the problem under the interviewer eyes. –  vals Feb 28 at 22:00

Overall, code is easy to read.

A few minor nit-picky items.

Try not us use booleans, use enums instead.

i.e.

public enum EmployeeStatus {
OnCall,
Available
}


This allows you to add more status as requirements change:

public enum EmployeeStatus {
OnCall,
Available,
OutToLunch,
OnVacation,
OnBreak
}


I don't like that getCallHandler() can populate the freshers. I would inject the list or maybe a factory that will be able to do that. This will decouple the code, and allow for much easier unit testing in the future.

I also don't like the set methods. Using inheritance should eliminate the need for that. Admittedly I don't see the solution right now, but there is one there. Basically by having different methods to handle different instances, you are tying yourself to those three types. What happens if you add a forth type, say ProductExpert? You now have to change this class to deal with it.

The canHandle method seems like it should be a calculation based off the status enum. This way you only have to set one flag in the class, not two.

public class Employee {

// code

public boolean isFree() {
return status == EmployeeStatus.Available;
}

// code
}

Fresher fresherTwo = new Fresher();
fresherTwo.setStatus(EmployeeStatus.Available);


I also don't like the name NoEmployeeInTheHouseException', I find it a little too causal. I would do something line 'NoEmployeeAvailableException' or something. It comes across as a little more businesslike.

-
Thanks for the great answer! –  Koray Tugay Feb 28 at 19:18
What do you mean I don't like that getCallHandler() can populate the freshers. ? This method does not populate freshers? –  Koray Tugay Feb 28 at 19:22
I think its bad practice when I class can populate itself. The analogy that is most used is that you don't expect a car to know how to build itself, you build it up with pieces using other machines... –  Jeff Vanzella Feb 28 at 19:31

First I will comment on your implementation, then I will propose my version at the end.

Here we go, from top to bottom:

public interface CallAllocator {
public Employee getCallHandler() throws NoEmployeeInTheHouseException;
void setPM(ProductManager productManager);
}

• "Employee" is too generic a term for somebody who handles calls. I would call it ICallHandler. The actual employee classes are irrelevant in the discussion of handling calls, they should not be part of the model design.

• The setTL, setPM, addFresher methods all use terms and arguments that are too implementation specific. An interface definition should be as abstract as possible.

Next:

if (freshers.isEmpty() && technicalLead == null && productManager == null) {
throw new NoEmployeeInTheHouseException();
}


I see that in your implementation you handle the availability of freshers/lead/manager as being empty or null. This is state management, not modeling. It would be better to capture the notion of being available or not by the model, using explicit interface methods.

if (!freshers.isEmpty()) {
Employee fresher = freshers.get(new Random().nextInt(freshers.size()));
if (fresher.getCanHandle()) {
return fresher;
}
}

• getCanHandle is really awkward, canHandle would be more natural
• The notion of getting the next available fresher deserves its own method: you can think of different implementations, such as pick any free fresher at random, or pick the least picked, or pick the most picked, and so on.

Ok so here's my solution to model the description quite accurately:

interface ITicket {}

interface ICallHandler {
boolean isAvailable();
boolean canHandle(ITicket ticket);
}

interface ICallHandlerPicker {
ICallHandler getAvailableCallHandler();
}

interface ICallCenter {
ICallHandler getCallHandler(ITicket ticket);
}

private final ICallHandlerPicker picker;
private final ICallHandler manager;

this.picker = picker;
this.manager = manager;
}

@Override
public ICallHandler getCallHandler(ITicket ticket) {
ICallHandler handler = picker.getAvailableCallHandler();
if (handler == null) {
// nobody available. perhaps throw new NoSuchElementException() ?
return null;
}
if (handler.canHandle(ticket)) {
return handler;
}
}
return manager;
}
}


This sticks to the well-defined parts of the description and leaves the undefined parts unimplemented on purpose, such as:

• If there are many free freshers, which one to pick? --> implement as you like
• If there are no free freshers, what to do? --> null implies that there's nobody to handle, though I admin I don't like this part much

Other things to note:

• The interfaces are short and to the point, with only getters, no mutators
• The class members are all final, and the class is fully defined at construction time, there's no room left for guessing
• In terms of handling calls, the tech lead and product manager are call handlers just like the freshers, so using interfaces makes good sense, no need to give them dedicated classes

Of course this is not scalable. The description itself excluded scalability by specifying a single tech lead and a single product manager. I would fix that by modeling the multiple levels as a chain of call centers:

class MultiLevelCallCenter implements ICallCenter {
private final ICallHandlerPicker picker;
private final ICallCenter nextCallCenter;

MultiLevelCallCenter(ICallHandlerPicker picker, ICallCenter nextCallCenter) {
this.picker = picker;
this.nextCallCenter = nextCallCenter;
}

@Override
public ICallHandler getCallHandler(ITicket ticket) {
ICallHandler handler = picker.getAvailableCallHandler();
if (handler == null) {
// nobody available. perhaps throw new NoSuchElementException() ?
return null;
}
if (handler.canHandle(ticket)) {
return handler;
}
return nextCallCenter.getCallHandler(ticket);
}
}

class UltimateCallCenter implements ICallCenter {
private final ICallHandler handler;

UltimateCallCenter(ICallHandler handler) {
this.handler = handler;
}

@Override
public ICallHandler getCallHandler(ITicket ticket) {
return handler.isAvailable() ? handler : null;
}
}


Then, we could implement the call center in the description in terms of these more scalable classes as:

ICallCenter getSingleLeadSingleManagerCallCenter(ICallHandlerPicker picker, final ICallHandler lead, ICallHandler manager) {
ICallCenter managerCallCenter = new UltimateCallCenter(manager);
ICallCenter leadCallCenter = new MultiLevelCallCenter(new ICallHandlerPicker() {
@Override
public ICallHandler getAvailableCallHandler() {
}
}, managerCallCenter);
}


Again, I left out the details of the freshers and how we pick them. These are not specified in the description, and not really relevant. This leaves you free to inject whatever implementation you like.

-
Clean Code by Robert C. Martin, Interfaces and Implementations, p24 has a good argument agains the I interface prefixes. "I don’t want my users knowing that I’m handing them an interface. I just want them to know that it’s a ShapeFactory." –  palacsint Mar 1 at 12:00
Hm, I guess that's a debatable subject. I dropped that point from my post, thanks. –  janos Mar 1 at 12:04

## Naming Issue

• CallAllocatorImpl have a group of freshers, TL & a PM. So I think naming it Office or CallCenter seems feasible.
• getCanHandle is misleading. You won't usually see methods name have get and can both. Secondly what to handle? Call. So canHandleCall seems reasonable.
• Same goes for setCanHandle.

There can be multiple employees, but only one TL or PM

Smelling a singleton-ish work.

So setTL needs few more LOC

public void setTL(TechnicalLead technicalLead) {

Same goes for setPM.
CallAllocatorImpl is the implementation of CallAllocator`, so if you changed the name of the implementation you need to change the name of the interface too. –  Marc-Andre Mar 1 at 1:49