Modelling a Call Center

Question

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 setTL(TechnicalLead technicalLead);
    void setPM(ProductManager productManager);
    void addFresher(Fresher fresher);
}

Implementation for the interface:

public class CallAllocatorImpl implements CallAllocator {

    private TechnicalLead technicalLead;
    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 (technicalLead != null && technicalLead.getCanHandle()) {
            return technicalLead;
        }

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

        throw new NoEmployeeInTheHouseException();

    }
    @Override
    public void setTL(TechnicalLead technicalLead) {
        this.technicalLead = technicalLead;
    }
    @Override
    public void setPM(ProductManager productManager) {
        this.productManager = productManager;
    }
    @Override
    public void addFresher(Fresher fresher) {
        if (fresher.isFree()) {
            freshers.add(fresher);
        }
    }
}

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);

        callAllocator.addFresher(fresherOne);
        callAllocator.addFresher(fresherTwo);
        callAllocator.addFresher(fresherThree);
        callAllocator.addFresher(fresherFour);

        TechnicalLead technicalLead = new TechnicalLead();
        technicalLead.setCanHandle(false);
        technicalLead.setFree(true);

        callAllocator.setTL(technicalLead);

        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?

Answer 1

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.

Answer 2

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.

This is a good start, and I'm glad you are concerned about this stuff. Keep working at it.

Answer 3

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 setTL(TechnicalLead technicalLead);
    void setPM(ProductManager productManager);
    void addFresher(Fresher fresher);
}

• "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);
}

class SingleLeadSingleManagerCallCenter implements ICallCenter {
    private final ICallHandlerPicker picker;
    private final ICallHandler lead;
    private final ICallHandler manager;

    SingleLeadSingleManagerCallCenter(ICallHandlerPicker picker, ICallHandler lead, ICallHandler manager) {
        this.picker = picker;
        this.lead = lead;
        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;
        }
        if (lead.isAvailable() && lead.canHandle(ticket)) {
            return lead;
        }
        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() {
            return lead.isAvailable() ? lead : null;
        }
    }, managerCallCenter);
    return new MultiLevelCallCenter(picker, leadCallCenter);
}

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.

Answer 4

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.

---

Answer the Question properly

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) {
    if(technicalLead == null) {
        this.technicalLead = technicalLead;
    }
    else {
        throw new TLAlreadyExistsException("Only one Team-Leader allowed").
    }
}

Same goes for setPM.

Answer 5

Naming: I don't know the Java coding convention but names of setters and getters such as setCanHandle() look a bit awkward to me.

In my opinion the logic whether an Employee can handle a call can be encapsulated by the class. In my own C# implementation I've decided to use Chain-of-responsibility pattern which looks more appropriate in this case.

CallHandler class:

public interface ICallHandler
{
    IResponse Handle(ICallContext callContext); 
    bool IsFree { set; get; }
    bool CanHandle { get; } 
}

public abstract class CallHandlerBase : ICallHandler
{
    private ICallHandler _successor;

    public CallHandlerBase(ICallHandler successor){
        _successor = successor;
    }

    //logic whether a call handler can handle a call
    public abstract bool CanHandle { get; }

    public virtual bool IsFree { set; get; }

    public virtual IResponse Handle(ICallContext callContext)
    {
        IResponse response;
        if (CanHandle && IsFree){
            //Handle logic
            response = new Response();
        } else {
            //escalate to the successor
            response = _successor.Handle(callContext);
        }
        return response;
    }
}

public class Fresher : CallHandlerBase{ }

public class TeamLead : CallHandlerBase{ }

public class ProductManager : CallHandlerBase
{ 
    //PM always can handle a call   
    public override bool CanHandle{
        return true;
    }
}

I would've also encapsulated the logic of getting a new fresher. In this case an ObjectPool to keep freshers looks very convenient.

public class CallCenter
{   
    private ICallHandler _teamLead;
    private ICallHandler _productManager;   
    private ObjectPool<ICallHandler> _freshersPool;

    public class CallCenter(ICallHandler productManager, ICallHandler teamLead, Func<ICallHandler> createFresher){      
        _freshersPool = new ObjectPool<ICallHandler> (createFresher);
    }

    public ICallHandler GetCallHandler(){
        fresher = _objectPool.GetObject();
        return fresher;
    }

    public void SetFree(ICallHandler handler){
        _freshersPool.PutObject(handler);
    }
}

Clients code:

var productManager = new ProductManager();  
var teamLead = new TeamLead(productManager);

var callCenter = new CallCenter(productManager, teamLead, () => new Fresher(teamLead));
var callHandler = callCenter.GetCallHandler();

var callContext = new CallContext();
var response = callHandler.Handle(callContext); 
callCenter.SetFree(callHandler);