SoulCub
SoulCub

Reputation: 467

Is it possible to rewrite such code in Java8 streams?

I was trying to adapt it to Java 8 streams:

public boolean isProcessionRestricted(CommonMessage message) {
    if (message.getClass() == BonusMessage.class) {
        log.debug("Staring validating BonusMessage: '{}'", message);
        BonusMessage bonusMessage = (BonusMessage) message;
        Optional<BonusTriggerConfig> config = bonusTriggerConfigRepository.getCached();
        if (config.isPresent()) {
            BonusTriggerConfig bonusTriggerConfig = config.get();
            List<BonusRewardConfig> rewardConfigs = bonusTriggerConfig.getRewardConfigs();
            if (!rewardConfigs.isEmpty()) {
                return rewardConfigs.stream()
                        .map(BonusRewardConfig::getBonusTypeId)
                        .noneMatch(bonusTypeId -> bonusTypeId == bonusMessage.getBonusTypeId());
            } else {
                return false;
            }
        } else {
            return false;
        }
    }
    return false;
}

but I faced the problem with checking if collection is empty in streams. The "streammest" thing which I get looks like this:

@Override
public boolean isProcessionRestricted(CommonMessage message) {
    if (message.getClass() == BonusMessage.class) {
        log.debug("Staring validating BonusMessage: '{}'", message);
        BonusMessage bonusMessage = (BonusMessage) message;
        return bonusTriggerConfigRepository.getCached()
                .map(bonusTriggerConfig -> {
                    List<BonusRewardConfig> rewardConfigs = bonusTriggerConfig.getRewardConfigs();
                    return !rewardConfigs.isEmpty() && rewardConfigs.stream()
                            .map(BonusRewardConfig::getBonusTypeId)
                            .noneMatch(bonusTypeId -> bonusTypeId == bonusMessage.getBonusTypeId());
                }).orElse(false);
    }
    return false;
}

but still I don't like it.

Upvotes: 3

Views: 127

Answers (2)

holi-java
holi-java

Reputation: 30676

You can use Optional#filter to filtering the empty collection instead, for example:

return bonusTriggerConfigRepository.getCached()
        .map(bonusTriggerConfig -> bonusTriggerConfig.getRewardConfigs())
        // v--- filter the empty configs out
        .filter(rewardConfigs-> !rewardConfigs.isEmpty())
        .map(rewardConfigs -> rewardConfigs.stream()
            .map(BonusRewardConfig::getBonusTypeId)
            .noneMatch(bonusTypeId -> bonusTypeId == bonusMessage.getBonusTypeId())
        )
        .orElse(false);

Upvotes: 4

123-xyz
123-xyz

Reputation: 637

Regardless @Joe C's comments, I'm not sure if it's better to move this OP to Code Review. But I learned something from OP about how to write concise code with Java 8 Stream API, First, Here is my first try with StreamEx (I didn't try with native stream APIs because it's too boring to me...)

public boolean isProcessionRestricted(CommonMessage message) {
    return StreamEx.of(message)
        .select(BonusMessage.class)
        .peek(m -> log.debug("Staring validating BonusMessage: '{}'", m))
        .anyMatch(m -> bonusTriggerConfigRepository.getCached()
                .map(btc -> StreamEx.of(btc.getRewardConfigs())
                        .noneMatch(brc -> brc.getBonusTypeId() == m.getBonusTypeId())).orElse(false));
}

(If there are some compile errors, please help to update my answer). But logic looks too complicate to me. Here are the codes I may write if I was the programmer:

public boolean isProcessionRestricted(CommonMessage message) {
    if (!(message insanceof BonusMessage && bonusTriggerConfigRepository.getCached().isPresent())) {
        return false;
    }

    log.debug("Staring validating BonusMessage: '{}'", message);     
    int restrictedBonusTypeId = ((BonusMessage) message).getBonusTypeId();
    List<BonusRewardConfig> rewardConfigs = bonusTriggerConfigRepository.getCached().get().getRewardConfigs();
    return rewardConfigs.size() > 0 && rewardConfigs.stream()
            .noneMatch(brc -> brc.getBonusTypeId() == restrictedBonusTypeId);
}

What I learned or suggest:

  1. Forget Stream API, It just looks cool, but not that cool. Writing concise code with/without Stream API is really cool.
  2. Although I like Lambdas and Stream API. However, how to write concise code with Stream API is much more challenge to me, comparing the for-loop/if/while. it's better to go over the Stream API again and again, and do more practices before you start to use the stream API in real product.
  3. Always prefer to StreamEx. Sometimes it's really boring to write code with native Stream API. StreamEx provides a lot of shorter and convenient ways to accomplish your tasks.

Forget stream

Upvotes: 0

Related Questions