Returning a URL string of an API endpoint

Question

I have a class with multiple methods which return a URL string of an API endpoint. Each method takes different parameters based on what the endpoint have to include.

Examples (among others):

1. GET: http://api.mydomain.com/v1/shopping_lists/items/categories
2. GET: http://api.mydomain.com/v1/shopping_lists/items?last_sync_date=2016-09-29T17%3A14%3A51Z&page_number=1&page_limit=10&include=categories%2Csubcategories
3. POST: http://api.mydomain.com/v1/users/{user_guid}/shopping_lists/{shopping_list_guid}/items
4. GET: http://api.mydomain.com/v1/users/guid?include=devices%2Cshopping_lists.items.images%2Cshopping_lists.occasions

As we can see in the URLs, I have query parameters as well as the HTTP method in which the API is getting, along with the body request in JSON or form-data.

---

This is my current approach on building those URL strings:

I'm using OkHttp as the HTTP client. Firstly, I've created an abstract class called BaseHttpCall. Inside, I put all of the HTTP methods with the Request Builder like so:

public abstract class BaseHttpCall {

    private static final String BASE_URL = "http://api.mydomain.com";
    private static final String VER = "/v1/";
    private static final String API_URL = BASE_URL + VER;

    public Request post(String endpoint, RequestBody requestBody, boolean auth) {
        return new Request.Builder()
                .url(API_URL + endpoint)
                .post(requestBody)
                .addHeader("Content-Type", "application/json")
                .addHeader("Authorization", (auth) ? "Bearer " + session.token().get() : "")
                .build();
    }

    public Request get(String endpoint, boolean auth) {
        return new Request.Builder()
                .url(API_URL + endpoint)
                .get()
                .addHeader("Content-Type", "application/json")
                .addHeader("Authorization", (auth) ? "Bearer " + session.token().get() : "")
                .build();
    }

    // other HTTP method builder ...

    public String buildQueryString(String endpoint, HashMap<String, String> keyValuePair) throws APIError {

        // process keyValuePair

        return mEndpointWithQueryString;
    }
}

For the endpoint management, I put it in a class called Endpoints that holds all of the endpoints:

public class Endpoints {

    public static final String DEVICES = "devices";
    public static final String USERS = "users";
    public static final String SHOPPING_LIST = "shopping_lists";
    public static final String SMS = "sms";
    // ... other endpoints


    /**
     * Build Shopping List endpoint for Create/View
     */
    public static String buildShoppingListEndpoint(String user_guid, String[] includes) {
        String mBaseEndpoint =
                USERS + "/" + user_guid + "/" +
                        SHOPPING_LIST;
        if (includes == null) {
            return mBaseEndpoint;
        }
        return mBaseEndpoint + include(includes);
    }

    /**
     * Build Shopping List endpoint for Update/Delete
     */
    public static String buildShoppingListEndpoint(String user_guid, String shopping_list_guid, String[] includes) {
        String mBaseEndpoint =
                USERS + "/" + user_guid + "/" +
                        SHOPPING_LIST + "/" + shopping_list_guid;
        if (includes == null) {
            return mBaseEndpoint;
        }
        return mBaseEndpoint + include(includes);
    }

    /**
     * Build User Shopping List Item endpoint
     *
     */
    public static String buildGroceryListEndpoint(String user_guid, String shopping_list_guid, String item_guid, String[] includes) {
        String mBaseEndpoint =
                USERS + "/" + user_guid + "/" +
                        SHOPPING_LIST + "/" + shopping_list_guid + "/" +
                        ITEMS + (StringUtils.isNotEmpty(item_guid) ? "/" + item_guid : "");
        if (includes == null) {
            return mBaseEndpoint;
        }
        return mBaseEndpoint + include(includes);
    }

    // ... other build methods

    /**
     * Construct the query parameter for requesting custom response
     */
    private static String include(String[] includes) {
        return "?include=" + StringUtils.join(includes, ",");
    }

    private static String slash() {
        return "/";
    }
}

In order to use these functions, I would do this - example for building Sub Category Item Endpoint (considering this function is inside a class which extends BaseHttpCall abstract method):

String[] includes = {"category", "items"};
HashMap<String, String> keyValuePair = new HashMap<>();
// ... key value pair builder here

Request request = 
    get(
        buildQueryString(
            Endpoints.buildSubCategoryItemEndpoint(user_guid, sub_category_id, null), keyValuePair), true);

I think this smells terrible behind the scenes. Are there any improvements I can do to make this more readable and maintainable in the backend, along with making the usage of the Endpoint builder more easily?

Answer 1

First of all I would get rid of the Endpoints class. You can encapsulate the individual endpoint configurations/formats in an inheritance hierarchy. This has the added benefit of not being publicly exposed should anything with the API change.

Secondly (and this is only a small thing) I would rename BaseHttpCall to something like MyApiRequest. If your domain name is let's say dutchcourage.com, then I would call the class DutchCourageApiRequest.

Here is the code implementing my suggested changes:

public abstract class DutchCourageApiRequest {

  private static final String BASE_URL = "http://api.dutchcourage.com";
  private static final String VER = "/v1/";
  private static final String API_URL = BASE_URL + VER;

  public abstract Request post(RequestBody requestBody, boolean auth);

  public abstract Request get(boolean auth);

  protected Request post(String endpoint, RequestBody requestBody, boolean auth) {
    return new Request.Builder()
        .url(API_URL + endpoint)
        .post(requestBody)
        .addHeader("Content-Type", "application/json")
        .addHeader("Authorization", (auth) ? "Bearer " + session.token().get() : "")
        .build();
  }

  protected Request get(String endpoint, boolean auth) {
    return new Request.Builder()
        .url(API_URL + endpoint)
        .get()
        .addHeader("Content-Type", "application/json")
        .addHeader("Authorization", (auth) ? "Bearer " + session.token().get() : "")
        .build();
  }

  // other HTTP method builder ...

  protected String buildQueryString(String endpoint, HashMap<String, String> keyValuePair) throws APIError {
    String mEndpointWithQueryString = null;
    // process keyValuePair
    return mEndpointWithQueryString;
  }

}

The sub category item concrete implementation.

public class SubCategoryItemApiRequest extends DutchCourageApiRequest {

  @Override
  public Request post(RequestBody requestBody, boolean auth) {
    return null;
  }

  @Override
  public Request get(boolean auth) {
    String[] includes = {"category", "items"};
    Map<String, String> keyValuePair = new HashMap<>();
    // ... key value pair builder here

    Request request =
        get(
            buildQueryString(
                buildSubCategoryItemEndpoint(user_guid, sub_category_id, null), keyValuePair), true);
  }

  private String buildSubCategoryItemEndpoint(String user_guid, String sub_category_id, String other) {
    // create url string
  }

}

Answer 2

You should remove all comments, since they only repeat what the method names say already.

You should remove the slash method since it is not used. And it should not be.

Instead of using string concatenation, you could use string formatting (only if you find it easier to read):

String url1 = "/" + user + "/" + guid + includes(includes);

Steing url2 = String.format("/%s/%s%s", user, guid, includes(includes));

The includes method should handle a null or empty array. This avoids code duplication in the code that calls this method.