2
\$\begingroup\$

here is my code to get quotes for forex pairs, using the CurrencyLayer API, with CompletableFuture. We do an HTTP GET, parse the JSON response and map it into my data structures (see sample output at the bottom)

Does this look OK? I'm worried about thread-safety !! :x

Currency.java:

public enum Currency {
    GBP("GBP", "Pound Sterling"),   
    EUR("EUR", "Euros"),
    USD("USD", "US Dollars");

    public final String symbol;
    public final String name;
    Currency(String symbol, String name) {
        this.symbol = symbol;
        this.name = name;
    }
}

CurrencyPair.java:

public class CurrencyPair {
    final public Currency base;
    final public Currency counter;

    public CurrencyPair(Currency base, Currency counter) {
        this.base = base;
        this.counter = counter;
    }
}

CurrencyLayer.java:

import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.CompletableFuture;
import javax.ws.rs.client.Client;
import javax.ws.rs.client.ClientBuilder;
import javax.ws.rs.core.Response;

import com.google.gson.JsonElement;
import com.google.gson.JsonObject;
import com.google.gson.JsonParser;
import models.Currency;

public class CurrencyLayer implements ForexQueryService {
    private final Client client = ClientBuilder.newClient();

    private CompletableFuture<Response> makeRequest(List<CurrencyPair> pairs) {
        final String url = "http://www.apilayer.net/api/live?access_key=[my key ^^]&format=1";
        return CompletableFuture.supplyAsync(() -> client.target(url).request().get());
    }

    private static double getUSDto(JsonObject quotes, Currency c) {
        if (c == Currency.USD) {
            return 1.0;
        } else {
            return quotes.get("USD" + c.symbol.replace("XBT", "BTC")).getAsDouble();
        }
    }

    private static List<ForexRate> convertResponse(Response r, List<CurrencyPair> pairs) {
        final String body = r.readEntity(String.class);

        // log this data
        System.out.println(body);

        JsonElement jelement = new JsonParser().parse(body);
        JsonObject jo = jelement.getAsJsonObject();
        JsonObject quotes = jo.get("quotes").getAsJsonObject();

        List<ForexRate> ret = new ArrayList<>();
        for (CurrencyPair p : pairs) {
            ret.add(new ForexRate(p.base, p.counter, getUSDto(quotes,p.counter)/getUSDto(quotes, p.base)));
        }

        return ret;
    }

    public CompletableFuture<List<ForexRate>> lookUpRates(List<CurrencyPair> pairs) {
        return makeRequest(pairs).thenApply((t) -> convertResponse(t, pairs));
    }
}

ForexRate.java

public class ForexRate {
    public final Currency from;
    public final Currency to;
    public final Double rate;

    public ForexRate(Currency from, Currency to, Double rate) {
        this.from = from;
        this.to = to;
        this.rate = rate;
    }
}

Main.java:

public class Main {
    static public void main(String[] args) {
        CurrencyLayer c = new CurrencyLayer();
        try {
            List<CurrencyPair> pairs = new ArrayList<>();
            pairs.add(new CurrencyPair(Currency.GBP,Currency.USD));
            pairs.add(new CurrencyPair(Currency.JPY,Currency.GBP));

            List<ForexRate> rates = c.lookUpRates(pairs).get();
            for (ForexRate rate : rates) {
                System.out.println(rate.from + " " + rate.to + " -> " + rate.rate);
            }

        } catch (Exception e) {
            e.printStackTrace();
        }
    }
}

Sample output:

{
  "success":true,
  "terms":"https:\/\/currencylayer.com\/terms",
  "privacy":"https:\/\/currencylayer.com\/privacy",
  "timestamp":1505384954,
  "source":"USD",
  "quotes":{
    "USDAUD":1.249706,
    "USDEUR":0.840197,
    "USDGBP":0.75723
  }
}
GBP USD -> 1.320602723082815
EUR GBP -> 0.9012529204460382
\$\endgroup\$

1 Answer 1

4
\$\begingroup\$

Thread-safety

I'm worried about thread-safety !! :x

I don't see any shared data anywhere, and as such there are no visible thread-safety issues.

If you called CurrencyLayer.lookUpRates from multiple threads, then CurrencyLayer.client would be shared and so it would need to be thread-safe.

The current program as it is, fetches and processes exchange rates practically sequentially, so there's no visible need for multi-threading, and doesn't really benefit at all from CompletableFuture. There is no visible reason for CurrencyLayer.lookUpRates to return CompletableFuture<List<ForexRate>>, it could as well return a simple List<ForexRate>.

Unused method parameter

private CompletableFuture<Response> makeRequest(List<CurrencyPair> pairs) {
    final String url = "http://www.apilayer.net/api/live?access_key=[my key ^^]&format=1";
    return CompletableFuture.supplyAsync(() -> client.target(url).request().get());
}

The pairs parameter is not used in this method.

Exception handling in main

In this code, it would be better to remove the try-catch. The outcome will look the same in case of failure, but the exit code will be correctly non-zero, which is currently not the case.

static public void main(String[] args) {
    CurrencyLayer c = new CurrencyLayer();
    try {
        // ...
    } catch (Exception e) {
        e.printStackTrace();
    }
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.