How can I replace the for loop with enhanced for loop in the following code?

Question

I have this code for sorting an arraylist

    ArrayList<Worker> workersSorted= sort(workers);
    System.out.println(workersSorted.toString());
}

public static ArrayList<Worker> sort(ArrayList<Worker> _workers) throws CloneNotSupportedException {
    Worker temp;
    for (int i = 0; i < _workers.size() - 1; i++) {
        if ((_workers.get(i)).compareTo(_workers.get(i+1)) == -1){
            temp = (Worker) _workers.get(i).clone();
            _workers.set(i, _workers.get(i+1));
            _workers.set(i+1, temp);    
        }
    }
    return _workers;
}

How may I rewrite the code using enhanced for loop?

I don't think you can use the enhanced for loop since the block inside your for loop depends on the indices of the workers that are next to each other and an enhanced for loop simply has access to a single worker in each iteration. — ruthless, Dec 05 '14 at 15:42
You can't as you are modifying the list, and not processing the last item, and you need the index. — Mark Rotteveel, Dec 05 '14 at 15:43
@pquest - you can use Google to find out - http://stackoverflow.com/questions/11685305/what-is-the-syntax-of-enhanced-for-loop-in-java — Betlista, Dec 05 '14 at 15:43
you can make an improvement by deleting `.clone()` there though. btw, u are not really sorting here. — Jason Hu, Dec 05 '14 at 15:44
btw: this is sort in O(n)? :-D From the code it seems, that Worker implements `Comparable`, why not to use just `Collections.sort()` ? — Betlista, Dec 05 '14 at 15:46
Don't. Enhanced for loop is meant to be used for read only purposes, and since you want to update collection you are iterating on you would get `ConcurrentModificationException`. Also `_workers.set` needs information of index of data you want to use (replace) so you still need some `i`. — Pshemo, Dec 05 '14 at 15:48

Rudi Kershaw · Answer 1 · 2014-12-05T16:01:42.023

Although this probably isn't the answer you are looking for, my opinion is that even if you could rewrite this method to use an enhanced for loop, you shouldn't.

Your current method is fairly clear in it's intention and an enhance for loop will not be able to clearly access Worker instances at specific indexes. Chances are you will likely also run into issues with ConcurrentModificationException if you try to make changes during the iteration. On top of that enhanced for loops are marginally (you could argue insignificantly) slower.

Essentially, an enhanced for loop is not well suited to your requirement.

score 0 · Answer 2 · answered Dec 05 '14 at 15:49

I think what you're looking for is this:

for(Worker w : _workers)

where you will use w instead of _workers.get(i) and _workers.get(_workers.indexOf(w) + 1) instead of _workers.get(i+1). As you can see, it will be quite ugly since you have to use indexOf in order to access the next element in the list and also check whether w is the last element of your list (because _workers.get(_workers.indexOf(w) + 1) will throw an error in that case). One other thing you need to consider - does your ArrayList contain duplicates? If it does, then indexOf will always return the first value it finds.

I suggest you stick with an ordinary for-loop in this case.

That's true. Even more reasons for which he should use an ordinary `for-loop`. I will add your point to my answer. — RockOnRockOut, Dec 05 '14 at 15:52

score 0 · Answer 3 · answered Dec 05 '14 at 15:52

0

Its pretty gross, but it should work. I shifted the indexes back, so now the i+1 is the current Worker object w and then I had to save the last worker in previous.

ArrayList<Worker> workersSorted= sort(workers);
    System.out.println(workersSorted.toString());}

    public static ArrayList<Worker> sort(ArrayList<Worker> _workers) throws        CloneNotSupportedException{

        Worker temp;
        Worker previous;
        int prevIdx=-1;
        ArrayList<Worker> wNew=new ArrayList<Worker>;

        for (Worker w: _workers){

            if (previous !=null && (previous).compareTo(w)==-1){
                temp=(Worker) w.clone();
                wNew.set(prevIdx, w);
                wNew.add( temp);    
            }
         previous=w;
         prevIdx++;
        }
        return _workers;
    }

answered Dec 05 '14 at 15:52

MiiinimalLogic

820
6
11

Looks like you'll run into concurrency issues. Will this actually run? – Rudi Kershaw Dec 05 '14 at 15:54
Can you elaborate on concurrency issues? I'm not sure how you mean this. – MiiinimalLogic Dec 05 '14 at 16:07
Nevermind, I meant `ConcurrentModificationException` but I didn't notice you were adding and setting to a new `Arraylist`. Your still returning the original list though, so your not actually returning a sorted list. >. – Rudi Kershaw Dec 05 '14 at 16:09

score 0 · Answer 4 · answered Feb 16 '23 at 13:20

0

The only enhancement I would do to your loop is before the loop assign _workers.size to a variable. Use the variable instead which removes the size check on every iteration.

answered Feb 16 '23 at 13:20

Robert Quinn

579
5
10

How can I replace the for loop with enhanced for loop in the following code?

4 Answers4