Best way to passing parameter to restTemplate.getForObject

Question

in order to write a clean and smart code, I'm wondering what can I do to improve my actual piece of code:

public JSONObject getCustomer(final String customerId) {
    if (customerId == null || customerId.equals("")) {
        return null;
    } else {
        final RestTemplate restTemplate = new RestTemplate();
        final String result = restTemplate.getForObject("http://localhost:6061/customers/" + customerId,
                String.class);
        return new JSONObject(result);
    }
}

Especially, I didn't like the way I composed the url, neither the check on customerId's value.

I'd like to have something like JPA, where I ask some information passing a parameter, just to be clear (in pseudocode):

public JSONObject getCustomer(final String customerId) {
    final RestTemplate restTemplate = new RestTemplate();
    final Query query = restTemplate.query("http://localhost:6061/customers/:customerId");

    query.addParameter("customerId", customerId);
    JSONObject result = query.getForObject();

    return result;
}

Then, if customerId would be null or some white spaces or not existing, I'd like that result would be null. Is there a way to do this with a standard library?

Thanks

Answer 1

Firstly I would rather use DTO objects to hold the response data and manipulate them rather than using a String representation of the payload. So you may change it like this. Here Jackson takes care of all the serialization and deserialization of your data.

CustomerDTO customerDTO = restTemplate
                    .getForEntity("http://localhost:6061/customers/{customerId}", CustomerDTO.class, customerId).getBody();

You can use javax.validators such as @Min, @NotEmpty etc at your controller to check for the empty values. A sample is given below.

@RequestMapping(value = someURL, params = {"id"})
public SomeResponse doSomething(@PathVariable(value = "id") @Size(min=1) String id)

This throws a ValidationException with a relevant error message which can be customized by you. You then need to have an error handling aspect that sets the error message in ErrorDTO object and set the status code appropriately.

Answer 2

First off, I would remove the else branch and refactor the condition to:

public JSONObject getCustomer(final String customerId) {
    if (isNull(customerId) || customerId.trim().isEmpty()) {
        return null;
    }
    ...
}

Second, if you have a bunch of URI variables, Spring guys recommend using a Map<String, String>:

final String templateURL = "http://localhost:6061/customers/{customerId}";
final Map<String, String> variables = new HashMap<>();

variables.put("customerId", customerId);
...

template.getForObject(templateURL, String.class, variables);

Third, the method shouldn't create a RestTemplate instance on its own. I would prefer injecting the already-tuned object into an instance field:

getTemplate().getForObject(templateURL, String.class, variables);

Finally, I would name the result more meaningful:

final String customerRepresentation = ...;

---

Some notes:

1. getCustomer actually returns a JSONObject, not a Customer.
2. templateURL hardcoded the base URL as well as the URL to customers.
3. The method does a lot of work (takes too much responsibility) - argument validation, URL construction, making a request. Try to split these responsibilities between corresponding methods.