2
\$\begingroup\$

I am new to Java and I use the following code to retrieve parameters from a linked hash map from the Cloud Foundry API. My question is if there is a better way to do it in Java?

public HashMap<String, String> getUserData(String Url, String org, String space, String app) throws MalformedURLException { 
        URL endpoint = new URL(Url);
        CloudFoundryOperations ops = sm.getCurrentUserCfClient(endpoint, org, space);

        GetApplicationEnvironmentsRequest request = GetApplicationEnvironmentsRequest.builder()
                .name(app)
                .build();

        ApplicationEnvironments environments = ops.applications()
                .getEnvironments(request)
                .block();

Map<String, Object> mapSystemProvided =  environments.getSystemProvided();

    LinkedHashMap<String, Object> SERVICES = (LinkedHashMap<String, Object>) mapSystemProvided.get("SERVICES");
    ArrayList<Object> apps = (ArrayList<Object>) SERVICES.get("apps");
    LinkedHashMap<String, Object> appsList = (LinkedHashMap<String, Object>) apps.get(0);
    LinkedHashMap<String, Object> credentials = (LinkedHashMap<String, Object>) appsList.get("credentials");

    HashMap<String, String> parameters = new HashMap<String, String>();
    parameters.put(ID, credentials.get(ID).toString());
    parameters.put(DEFAULTS, credentials.get(DEFAULTST).toString());

    return parameters; 
\$\endgroup\$
2

1 Answer 1

2
\$\begingroup\$

Consider breaking this into more, reusable methods. E.g.

public Map<String, Object> getCredentials(String url, String org, String space, String app)
        throws MalformedURLException {

    GetApplicationEnvironmentsRequest request = GetApplicationEnvironmentsRequest.builder()
            .name(app)
            .build();

    Map<String, Object> mapSystemProvided = sm
            .getCurrentUserCfClient(new URL(url), org, space)
            .applications()
            .getEnvironments(request)
            .block()
            .getSystemProvided();

    Map<String, Object> SERVICES = (LinkedHashMap<String, Object>) mapSystemProvided.get("SERVICES");
    List<Object> apps = (ArrayList<Object>) SERVICES.get("apps");
    Map<String, Object> appsList = (LinkedHashMap<String, Object>) apps.get(0);

    return (LinkedHashMap<String, Object>) appsList.get("credentials");
}

public Map<String, String> getUserData(String url, String org, String space, String app)
        throws MalformedURLException {

    Map<String, Object> credentials = getCredentials(url, org, space, app);

    Map<String, String> parameters = new HashMap<>();
    parameters.put(ID, credentials.get(ID).toString());
    parameters.put(DEFAULTS, credentials.get(DEFAULTS).toString());

    return parameters;
}

Your original method was mostly getting the credentials. If we move that into another method, it actually leaves this method rather simple and straightforward.

I moved what had been ops into the mapSystemProvided chain, as it wasn't used for anything else.

As a general rule, it is easier to have interfaces as types rather than implementations. In this case, it makes the lines fit in the code window without scrolling on my screen. That may not matter in your IDE though. Mostly I just find it a good habit. Most all of the time, you're just using the interface methods anyway. Using the interface as the type helps enforce that and allows you to switch implementations easily. Even if it doesn't actually work that way here.

I think that you could change the casts to the interface as well. I didn't try that here, as I don't work with casting enough to be sure. And of course there isn't enough context here to actually test this code. But if I was working on this code, I'd try that to decouple it even more from the implementations.

I'd probably change SERVICES to services as I changed Url to url. That way they fit the standard Java naming scheme.

I would consider changing the entity model. For example, why is credentials hidden multiple levels deep inside of the getSystemProvided() result? Maybe there's a reason for that, but it's hard to see with just this code. It seems overly abstracted, at least when viewed in isolation like this.

Your indentation is funky. I fixed it to be consistent in the alternative versions. Perhaps that was just a copy & paste error. There was also a missing } that I added.

\$\endgroup\$
1
  • \$\begingroup\$ If the types are being relaxed, for example from LinkedHashMap<String, Object> SERVICES to Map<String, Object> SERVICES, then the casts should be relaxed as well. \$\endgroup\$ Commented Apr 6, 2017 at 23:41

Your Answer

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

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