Multiple If Statement Reduction

Viewed 259

I'm looking for some help cleaning up the code below and reducing the number of lines. Is there a way to not set anything if the get returns a null?

            if (map.get("cpn_rate") != null) {
                collateral.setCoupon(new BigDecimal(map.get("cpn_rate")));
            }
            if (map.get("Price") != null) {
                collateral.setPrice(new BigDecimal(map.get("Price")));
            }
            if (map.get("Par") != null) {
                collateral.setPar(new BigDecimal(map.get("Par")));
            }
            if (map.get("mkt_val") != null) {
                collateral.setMarketValue(new BigDecimal(map.get("mkt_val")));
            }
            if (map.get("Accrued Intr") != null) {
                collateral.setAccurInterest(new BigDecimal(map.get("Accrued Intr")));
            }
            if (map.get("Total Market Value") != null) {
                collateral.setTotMktValue(new BigDecimal(map.get("Total Market Value")));
            }
5 Answers

The simple answer to the overt question of, "can I make this more concise/terse" is "no". You're not really going to get what you're looking for in making this more terse or concise, nor would computeIfPresent really give you what you're looking for and keep your code readable.

The issue is that, when you go to retrieve a key from your map, you're putting it in a different field in your collateral instance. This means that trivial solutions such as looping over the map won't satisfy since you're not going to be able to get the exact field you need to map to without getting deep into reflection.

The code you have here, albeit verbose, is perfectly readable and reasonable to any other maintainer to understand what's going on. I see no incentive to change it.

I'll throw in another option since there's more discussion here than I thought there would be.

Sometimes it makes sense to flip logic like this on its head (it may or may not in this case). Instead of having this chunk of code in the mainline code, move it into Collateral (or whatever the classname is).

The mainline code would then get cooked down to something like:

collateral.fillFromData(map); // TODO: Name something that makes sense in your domain

Or it could be a factory method, or whatever–context would determine the best approach(es).

The Collateral class would encapsulate the logic that determines which values are filled, and how those values are filled, based on the data passed in. That logic would likely be identical to what's here already (or a brief refactoring to wrap it up as in k5_'s second solution (which I don't have a problem with despite some localized complexity).

This localizes initialization/filling logic to the class that owns that data: this may or may not be reasonable. If it isn't reasonable then it's likely there's a missing helper/service/utility class somewhere in here that makes sense in the context of where/how the original code is invoked.

If it is no problem to actually set null and you just want to avoid the npe creating the BigDecimal. I would go with this helper method.

BigDecimal getBigDecimalOrNull(Map<String,String> map, String key){
    String value= map.get(key);
    if (value == null){
       return null;
    }
    return new BigDecimal(value);
}

And use it with

collateral.setTotMktValue(getBigDecimalOrNull(map, "Total Market Value"));

If the setting null is an actual problem. You can use this helper method

void setBigDecimalWhenNotNull(Map<String,String> map, Consumer<BigDecimal> consumer, String key){
    String value= map.get(key);
    if (value == null){
       return null;
    }
    consumer.accept(new BigDecimal(value));
}

and use it with:

 setBigDecimalWhenNotNull(map, ColleralClass::setToMktValue, "Total Market Value")

You can use Jackson Mapper to convert a Map to a POJO, but to be able to do that, you'll need to follow naming convention. The field name in POJO has to match with the key in your Map.

import org.codehaus.jackson.map.ObjectMapper;

final ObjectMapper mapper = new ObjectMapper();
final Collateral collateral = mapper.convertValue(map, Collateral.class);
Related