Bitcoin trading system

Recently I did an exercise for a interview pre-screening, but was not selected. They did not give much detailed feedback apart from "the code was not up to what they wanted".

It would be really helpful to understand as to whether the code written is up to production standards and what is it that I could be missing.

Here are the requirements:


Our business wants to move in the bitcoin market and we’ve been asked
to implement the first cut of a simple RFQ (request-for-quote)
service. The API looks like this and cannot be changed

public interface RfqService {
    Optional quoteFor(String currency, int amount);
}

public class Quote {
    public final double bid;
    public final double ask;
    public Quote(double bid, double ask) {
        this.bid = bid;
        this.ask = ask;
    }
}

The job of our engine is to respond with a bid (the price we buy at)
and ask (the price we sell at) by finding an exact client offer to
match on each side. To do so, our service has access to the company’s
live order board which given a single payment currency (e.g. USD) will
return a list of clients’ pending orders.

public interface LiveOrderBoard {
    List ordersFor(String currency);
}

The response from our service should be a single bid/ask quote. If the
request cannot be fulfilled on both sides by our internal RFQ service,
the response should be empty.

Bid = highest buy order – X
Ask = lowest sell order + X

The difference between the client order and our quote (i.e. the X
amount) is how we make money. X is 0.02 irrespective of the currency.

Worked examples


RFQ for 200 bitcoins paying in USD should return:

Bid (we buy) = 232.69
Ask (we sell) = 232.75

RFQ for 500 bitcoins cannot be fulfilled because there’s no matching
Sell order.

The solution


Please provide an implementation of the RFQ engine in Java which will
allow the business to enter the bitcoin market. No UI or

In these screening situations, you only get a few seconds to make a good first impression. Unfortunately, this line already makes a bad impression:

public class RFQServiceEngine implements RfQService {

The inconsistent capitalization shows a lack of attention to detail. If the problem explicitly states that you have to implement the given RfqService interface exactly, you can't write implements RfQService. And the class should be named RfqServiceEngine.

The LiveOrderBoard should be passed in like this:

private final LiveOrderBoard orders;

public RfqServiceEngine(LiveOrderBoard liveOrderBoard) {
    if (liveOrderBoard == null) {
        throw new NullPointerException();
    }
    this.orders = liveOrderBoard;
}

Marking the LiveOrderBoard as a final field makes it clear that it cannot be swapped out after the constructor. I don't think it makes any sense to allow it to be null, and it makes even less sense to decline all future quotes as a result of it being null. (The last thing a financial firm wants is to lose millions of dollars due to a hidden bug. If something is wrong, just make it fail fast.)

Don't write worthless comments like this — it's a dead giveaway that you are a beginner.

//reference to the live order board

It is probably good that you used BigDecimal to represent money, to avoid floating-point imprecision. You didn't have to do it, since the specified interface has doubles. Personally, if I were given this problem for an interview, I would just go with double, but write a big comment like this in protest:

// FIXME: Money amounts should ideally be represented using BigDecimal,
// since doubles can lead to rounding errors.  Unfortunately, we are
// stuck with doubles since that is what the Quote class uses.
private static final double commission = 0.02;

However, having chosen to use BigDecimal, you screwed it up by writing new BigDecimal(0.02) instead of new BigDecimal("0.02"). The JavaDoc warns you that the BigDecimal(double) constructor is unpredictable, but you should be able to figure out yourself that writing 0.02 already risks losing precision.

Here also, // Our commission is a superfluous comment.

The question simply represented currencies as strings. What is this TradeCurrency class?

final TradeCurrency cur = TradeCurrency.valueOf(currency);

Did you include the TradeCurrency class with your submission? Why did you invent something to overcomplicate a simple problem?

You calculated the bid and ask prices, but did you actually check that the spread yields a positive profit? That may be the logic flaw that doomed your chances with the company — imagine how much money the firm could lose by accidentally making unprofitable trades!

StackExchange Code Review Q#129744, answer score: 7