Multiple If Statement Reduction

Question

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")));
            }

Answer 1

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.

Answer 2

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.