0

I want to switch the position of elements in my list based on a rule.

I have a Map which has some data:

Map<String,List<Person>> workDivision = [:]
//inserted data to map...

I'm looping the map and based on a rule I want to move the element in the list to back of the list.So I'm first removing the element and then inserting it again.

workDivision.each {String division, List<Person> list -> {
   if(list.size>1 && someRule = true) {
     for(int i = 0; i<list.size; i++) {
       Person p = list.get(i)
       list.remove(i)
       list.add(p)
     }
   }

}

The above code is not working and I don't know why. Can anybody check if I'm missing something or completly doing it wrong?

John
  • 447
  • 1
  • 8
  • 22
  • 2
    In what way, it is not working? Does it give an error, or the output is unexpected or something else? – jrook Nov 27 '19 at 17:41
  • 1
    What do you mean by "not working"? If you're not getting the expected output, what is happening instead? – Thomas Nov 27 '19 at 17:41
  • 1
    Based on your code without knowing what exactly error you are facing, it seems like you are remove and add item in list inside a for loop, which can cause problem also. plz check https://stackoverflow.com/questions/223918/iterating-through-a-collection-avoiding-concurrentmodificationexception-when-re – Yunhai Nov 27 '19 at 17:42
  • Sorry for not clarifying the output i'm getting. I'm gettng nothing. Empty result – John Nov 27 '19 at 17:43
  • so maybe there is no data in map... – daggett Nov 27 '19 at 17:54
  • Since there is an accepted answer showing the real problem, just a note on the source: your each there is basically a no-op. The `...each{ ... -> { ... }}` means, you are iterating the map and create a closure for each element (which get's thrown away since `each` is for sideeffects) – cfrick Nov 28 '19 at 10:45

1 Answers1

1

Your empty result is likely due to a scope issue with modifying the result of your iterator and not your actual object. I don't know groovy enough to help that issue, so this answer will instead address the actual modification algorithm you're trying to use.

Your loop is moving multiple items. First it removes index 0 and puts it back on the end, then i increments and it moves index 1. What's in index 1 is NOT the next item in the list because you just shifted it.

Example list A, B, C, D, E

i=0   A is removed and added -> B, C, D, E, A
i=1   C is removed and added -> B, D, E, A, C
i=2   E is removed and added -> B, D, A, C, E
i=3   C is removed and added -> B, D, A, E, C
i=4   C is removed and added again -> B, D, A, E, C
i=5   loop ends
Thomas
  • 5,074
  • 1
  • 16
  • 12
  • Thanks, could I ask you to show how the code should be modified in java? – John Nov 27 '19 at 18:14
  • Your already have the right code to move an item to the end of the list. Your issue is that you have a loop which is doing the same move to half of the items in your list. – Thomas Nov 27 '19 at 18:17
  • How can I fix my loop? – John Nov 27 '19 at 18:34
  • If you're only trying to move one item, then don't have a loop at all. Loops are for when you're trying to perform an operation on multiple entries. – Thomas Nov 27 '19 at 18:40
  • I'm trying to check each item in the list and move them to the back someRule is true. If not then I do nothing – John Nov 27 '19 at 18:47
  • If that's the case then your if statement needs to be inside the loop. Right now if someRule is true it operates on the whole list. – Thomas Nov 27 '19 at 18:50