How to convert a stream to enhanced loop

Question

I have started working with Java 8 and trying to convert some stream to enhanced loop.

So, for the example, I'm trying to convert this stream to for loop. but, I'm not getting it right:

List<Job> jobs = shipment.getJobs();
Job shoppingJob = jobs.stream().filter(job -> job.isShopping()).findFirst().orElse(null);
Job deliveryJob = jobs.stream().filter(job -> job.isDelivery() || job.isRanger()).findFirst().orElse(null);

I want to know if it is possible to convert the above example to enhanced for loop. so far, I try to convert my for-loop code like this:

 List<Job> jobs = shipment.getJobs();
 Job shopping job = null;
 Job delivery job = null;
 Job rangerJob = null;
    if(jobs != null || jobs.isEmpty()) {
        for (Job job: jobs) {
            if (job.isShopping()) {
                shoppingJob = job;
            } else {
                if (job.isDelivery()) {
                    deliveryJob = job;
                }
            }
        }
    }

Answer 1

I can see a couple of issues with the code you've posted. This is the code that should do what you're after

    if(jobs != null) {
        for (Job job : jobs) {
            if (shoppingJob == null && job.isShopping()) {
                shoppingJob = job;
            } 
            else if (deliveryJob == null && (job.isDelivery() || job.isRanger())) {
                deliveryJob = job;
            }
        }
    }

So the things that needed to change:

1). The second part of your very first if condition would throw a NullPointerException if the jobs list was null. You had

if(jobs != null || jobs.isEmpty()) {

Which translates to: if jobs isn't null go into the block, or if jobs is null call isEmpty() on jobs (and calling isEmpty() on null will throw a NullPointerException). The isEmpty() is also redundant, as you pointed out in comments.

Instead, I think this is all you need

    if(jobs != null) {

2). You need to make sure the variables shoppingJob and deliveryJob don't get overwritten once they've been assigned a value once, if you want to have something equivalent to findFirst() from the Stream code.

Otherwise if there's more than one shopping job or more than one delivery job in the list, those variables will actually end up assigned the last matching job in the list and not the first. This is why the first part of these conditions is necessary i.e. in this example, the shoppingJob == null bit

if (shoppingJob == null && job.isShopping()) {

3). There's no need to write an if nested inside an else clause, you can just use else if if you need a conditional else statement. Although in this case, to match the logic exactly, you shouldn't use else at all (in the original Streams code, if a job returned true for isShopping and isDelivery, the same job could be assigned to both variables, but if you use an else here that could only ever be assigned to one)

So instead of this

else {
          if (deliveryJob == null && (job.isDelivery() || job.isRanger())) {

You can just use an if statement (with no else)

if (deliveryJob == null && (job.isDelivery() || job.isRanger())) {

4). Lastly, in the original code, you are trying to replicate, deliveryJob was assigned to a job that returned true for either isDelivery() or isRanger(). In your for loop code, you were only checking isDelivery()

Hope that helps.

Answer 2

Solved. Thanks

I just need to put null checking in each condition for shoppingJoband deliveryJob. It will be:

 List<Job> jobs = shipment.getJobs();
    Job shoppingJob = null;
    Job deliveryJob = null;
    Job rangerJob = null;
    for (Job job : jobs) {
        if (shoppingJob == null && job.isShopping()) {
            shoppingJob = job;
        }
        if (deliveryJob == null && (job.isDelivery())) {
            deliveryJob = job;
        }
    }

Answer 3

You should use break for findFirst function.

Below part is finding shoppingJob with breaking.

Job shoppingJob = null;
if (jobs != null || jobs.isEmpty()) {
    for (Job job : jobs) {
        if (job.isShopping()) {
            shoppingJob = job;
            break;
        }
    }
}

Below part is finding deliveryJob with breaking

Job deliveryJob = null;
if (jobs != null || jobs.isEmpty()) {
    for (Job job : jobs) {
        if (job.isShopping() || job.isRanger()) {
            deliveryJob = job;
            break;
        }
    }
}

But you wants to find with one foor loop , you can also do like this;

if (jobs != null || jobs.isEmpty()) {
    for (Job job : jobs) {
        if (!shoppingJobFound && job.isShopping()) {
            shoppingJob = job;
            shoppingJobFound = true;
        }
        if (!deliveryJobFound && (job.isDelivery() || job.isRanger())) {
            deliveryJob = job;
            deliveryJobFound = true;
        }
        if (shoppingJobFound && deliveryJobFound) {
            break;
        }
    }
}

Answer 4

The point of findFirst is that the Stream pipeline will only evaluate elements of the Stream until it finds the first matching element (that passes your filter).

When you convert that to a for loop, you should break from the loop once you find a match. However, since you are transforming two Stream pipelines to a single for loop, you should only break from the loop after you find matches for both conditions.

List<Job> jobs = shipment.getJobs();
Job shoppingJob = null;
Job deliveryJob = null;
if (jobs != null) {
    for (Job job : jobs) {
        if (shoppingJob == null && job.isShopping()) { // this is the first shopping job
            shoppingJob = job;
            if (deliveryJob != null) { // we can break after finding a match for both conditions
                break;
            }
        }
        if (deliveryJob == null && (job.isDelivery() || job.isRanger())) { // this is the first delivery or range job
            deliveryJob = job;
            if (shoppingJob != null) { // we can break after finding a match for both conditions
                break;
            }
        }
    }
}