2
\$\begingroup\$

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月29日T17%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?

200_success
146k22 gold badges190 silver badges479 bronze badges
asked Nov 17, 2016 at 7:22
\$\endgroup\$

2 Answers 2

3
\$\begingroup\$

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
 }
}
answered Dec 10, 2016 at 17:58
\$\endgroup\$
3
\$\begingroup\$

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.

answered Dec 11, 2016 at 10:33
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.