3

In the following method I have an ArrayList of Strings.

I check if any of these strings are in a txt file (checkIfWordIsInTextFile method).

If a string is in this list, I want to place it last in the List, then return the re-ordered list.

I am trying to use the set method but I'm not sure how to set the string to be the last element in the list.

current method:

 public List<String> placeParentsLastInLineItemList(List<String> listToReorder){

    for(String string: listToReorder){

       if(checkIfWordIsInTextFile(string)==true){
          listToReorder.set(,string); //how to set the string to the last element in list?
       }

    return listToReorder;
 }
Ross Drew
  • 8,163
  • 2
  • 41
  • 53
java123999
  • 6,974
  • 36
  • 77
  • 121

4 Answers4

7

Simplest Solution (for a beginner)

The ArrayList and List default add() behavior by definition, adds to the end of the list. So:-

listToReorder.add(string)

You will need to remove it from it's original position though, so don't use a for-each loop, use an index, then when you find it:-

listToReorder.remove(i);

...before re-adding it to the end, so:-

for (int i=0; i<listToReorder.size(); i++){
  String item = listToReorder.get(i);

//NOTE: '==true' isn't required if the method returns a boolean
  if(checkIfWordIsInTextFile(item)){ 
//As per  @rishi007bansod suggestion: If we remove an item, then the next item will be shifted into this index & effectively skipped.  Therefore we want the index not to change this time around so decrement i after usage
    listToReorder.remove(i--);
    listToReorder.add(item);
  }
}

Other options

  • You could create a new temporary list and recreate the list in the order you want but that makes for complicated code, as per @Andriy Kryvtsuns answer. The other problem with this is that you don't know what implementation of List was passed in, creating a whole list could be inefficient if, say, a LinkedList was passed it and remove() can run faster than linear time.

More advanced options

  • You could use a for-each loop (like @Ash French) but you'd need to use an Iterator as for-each loops restrict you from editing Collections you are looping over

  • You could get all Java 8 and use a lambda, as per @Mad Matts elegant solution

Ross Drew
  • 8,163
  • 2
  • 41
  • 53
  • 1
    @RossDrew this seems a good answer, can you please show how I do this all in the indexed for loop? – java123999 Oct 07 '16 at 14:21
  • 1
    @Ross Simplest Solution will fail where >=2 consecutive elements in list are present in text file. It will fail as list shifts elements after removing elements and we are always increasing index while looping. – rishi007bansod Dec 30 '19 at 09:18
  • 1
    @rishi007bansod well spotted. Updated my answer. – Ross Drew Dec 31 '19 at 13:58
2

As mentioned already you could removethe first occurrence of your string in the list and then add it, or create a new List. But it's also possible to sort your list, so that all words, which are in the text file appear last:

listToReorder.sort((s1, s2) -> {
    if (checkIfWordIsInTextFile(s1))
        return 1;
    return -1;
});
Mad Matts
  • 1,118
  • 10
  • 14
1

What I would do would be to use an iterator, then remove the item, and them add them at the end.

List<String> toAdd = new ArrayList<>();
Iterator iter = listToReorder.iterator();
while(iter.hasNext()){
   String string = iter.next();
   if(checkIfWordIsInTextFile(string)){
       toAdd.add(string);
       iter.next();
   }
}

listToReorder.addAll(toAdd);
Ash
  • 2,562
  • 11
  • 31
-1

Use a new List instance as a result:

public List<String> placeParentsLastInLineItemList(List<String> listToReorder) {
    List<String> inFileList = new ArrayList<>();
    List<String> notInFileList = new ArrayList<>();
    for (String string : listToReorder) {
        (checkIfWordIsInTextFile(string)? inFileList: notInFileList).add(string);
    }
    return joinLists(notInFileList, inFileList);
}

private List<String> joinLists(List<String> list1, List<String> list2) {
    List<String> result = new ArrayList<>(list1);
    result.addAll(list2);
    return result;
}

Advantage of this solution is also absence of a side effect in modification of input parameter listToReorder.

Andriy Kryvtsun
  • 3,220
  • 3
  • 27
  • 41
  • 1
    create a whole Collection, just to move an item to the end of an existing one? – Ross Drew Oct 07 '16 at 14:21
  • @RossDrew can't see any problem with this. But this solution with `foreach` more clear than without it. It's not necessary to do preliminary optimization without real reason. – Andriy Kryvtsun Oct 07 '16 at 14:23
  • This isn't optimization, it's just good coding. Creating an entire copy of a Collection just to move a single item is wasteful. – Ross Drew Oct 07 '16 at 14:26
  • Simple is deleting an element and adding it to the end. This is complex. So complex in fact, that you managed to mess up the logic. It doesn't work. It builds a string containing only the elements from the file. – Ross Drew Oct 07 '16 at 14:32
  • Still convinced this is the simpler solution? – Ross Drew Oct 07 '16 at 14:34
  • @RossDrew I fixed my solution. Still don't see any reason to work with input list doing side effect. – Andriy Kryvtsun Oct 07 '16 at 14:39
  • Fixed it but didn't you say "Good code is simple code", it's no longer simple. – Ross Drew Oct 07 '16 at 14:40
  • @RossDrew but it's still well understandable and without any side effects. It's usually better then modifying input params. – Andriy Kryvtsun Oct 07 '16 at 14:43
  • This is a piece of learning code, do you think thread access to this collection is going to be an issue? Didn't you say "It's not necessary to do preliminary optimization without real reason" You've written code twice as long with an extra method to catch a "side effect" that may be relied upon in program logic, the "spec" does say "then return the re-ordered list." not "return a new list" – Ross Drew Oct 07 '16 at 14:49
  • @RossDrew 1. I always write real (non learning) code 2. I haven't mentioned any thread related issues 3. I haven't caught `side effect` specifically. It's a result of good code practices. 4. Initial method signature has return param. – Andriy Kryvtsun Oct 07 '16 at 14:55
  • Return param doesn't state "new object" and the only "side effect" you could be referring to is thread collision. Either way, in an attempt to write the least complicated code, you've written the most complicated code. – Ross Drew Oct 07 '16 at 15:06