3
\$\begingroup\$

I recently had a programming interview, where I had to create an Api Gateway. The description is as follows:

You need to create a gateway in which it only allows certain number of callers to call api and rest of them will be blocked within the given timeframe.

For eg : If the time window is 1 minute. You need to allow only 6 calls to be called within that 1 minute window and rest of them will be blocked and will be only allowed in the next window of 1 minute.
In short every window will only allow 6 api calls.

I was not able write a proper code during 30 min round, since my brain started rushing towards all kinds of solutions.
But I tried to do it once I was done with an interview and below is my solution. Please let me know if there is anyway that I can improve in the implementation, in terms of performance or any other aspects.

public class APIGateway {
    private Employee emp;
    private int count = 0;
    private Timer timer;

    public APIGateway(Employee emp) {
        this.emp = emp;
        this.timer = new Timer(this);
        Thread t = new Thread(timer);
        t.start();
    }

    public synchronized void create() {
        while (!isAccessible()){
            try {
                wait();
            } catch (InterruptedException e) {
                e.printStackTrace();
            }
        }
        emp.create();
        count++;
    }

    private synchronized boolean isAccessible() {
        if(count<6){
            return true;
        }
        return System.currentTimeMillis()<=timer.getElapsetime();
    }

    private synchronized void unblock(){
        count = 0;
        notifyAll();
    }

    private class Timer implements Runnable {
        private long startTime;
        private APIGateway gateway;

        Timer(APIGateway gateway){
            startTime = System.currentTimeMillis();
            this.gateway = gateway;
        }

        private long getElapsetime() {
            return startTime + 6000;
        }

        @Override
        public void run() {
            while (true) {
                try {
                    Thread.sleep(60000);
                    startTime = System.currentTimeMillis();
                    gateway.unblock();
                } catch (InterruptedException e) {
                    e.printStackTrace();
                }
            }
        }

    }

    public void delete() {
        // same like create
    }

    public void update() {
        // same like create
    }
}

// Assume this is the api calls.
interface Employee {
    void create();
    void delete();
    void update();
}
\$\endgroup\$
2
\$\begingroup\$

My first point would be that if I want throttling for my API, I would put it in some kind of interceptor. However since this is an interview assignment and for the sake of simplicity I would recreate that as a function call:

public synchronized void create() {
    if(isAccessible()) {
        create(employer);
    }
}

Secondly, I don't think it is a good idea to block with the while-loop you have, when the quota of calls have been made. Instead return a meaningful message that explains why the request was denied.

With the current implementation you are using the InterruptedException as part of your control flow, which is usually considered an anti-pattern https://softwareengineering.stackexchange.com/questions/189222/are-exceptions-as-control-flow-considered-a-serious-antipattern-if-so-why.

I feel like a simpler solution would be to simply reset the counter every 60 seconds in the Timer class, similar to what you are already doing. Maybe I am missing something, but I don't understand why you are doing the check:

System.currentTimeMillis()<=timer.getElapsetime();

You are already resetting the counter every 60 seconds, so I would just return false in this method.

My last point would that generally the client would be passing Employee (or whatever data needed to create an employee) in the request, so it does not make sense to have Employee as a field. Rather it should be a parameter of the create function.

If this came as an interview question I would also see it as an opportunity to talk about throttling, and what alternatives there are. For example, maybe checking the rate of calls instead of having a fixed window of 60 seconds would make sense. However that's not that related to this code review.

\$\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.