1

I am trying to write a method that takes an ArrayList of Strings as a parameter and that places a string of four asterisks in front of every string of length 4.

However, in my code, I am getting an error in the way I constructed my method.

Here is my mark length class

import java.util.ArrayList;


public class Marklength {

    void marklength4(ArrayList <String> themarklength){
        for(String n : themarklength){
            if(n.length() ==4){
                themarklength.add("****");
            }
        }
        System.out.println(themarklength);
    }

}

And the following is my main class:

import java.util.ArrayList;


public class MarklengthTestDrive {
    public static void main(String[] args){

        ArrayList <String> words = new ArrayList<String>(); 

        words.add("Kane");
        words.add("Cane");
        words.add("Fame");
        words.add("Dame");
        words.add("Lame");  
        words.add("Same");

        Marklength ish = new Marklength();

        ish.marklength4(words);

    }
}

Essentially in this case, it should run so it adds an arraylist with a string of "****" placed before every previous element of the array list because the lengths of the strings are all 4.

BTW

This consists of adding another element

I am not sure where I went wrong. Possibly in my for loop?

I got the following error:

Exception in thread "main" java.util.ConcurrentModificationException
    at java.util.AbstractList$Itr.checkForComodification(AbstractList.java:372)
    at java.util.AbstractList$Itr.next(AbstractList.java:343)
    at Marklength.marklength4(Marklength.java:7)
    at MarklengthTestDrive.main(MarklengthTestDrive.java:18)

Thank you very much. Help is appreciated.

8
  • Please post your error as well.
    – Makri
    Mar 28, 2014 at 0:03
  • Read the javadoc of List#add. Then look for its overloaded method. Mar 28, 2014 at 0:04
  • 1
    You are attempting to modify a temporary variable created just for that foreach loop: stackoverflow.com/questions/85190/…, docs.oracle.com/javase/1.5.0/docs/guide/language/foreach.html Mar 28, 2014 at 0:06
  • You may not change a List while iterating it with for ( : ). Try with for (int i = 0; i < themarklength.size(); i++) and increment i inside the loop every time you add the "****".
    – andy
    Mar 28, 2014 at 0:11
  • 1
    Do you want to append four asterisks in front of a string of length four, or do you want to add another element to the list in front of the string? So, if you have ["item"], do you want it took look like ["****item"], or ["****", "item"]? Mar 28, 2014 at 0:22

6 Answers 6

8

Let's think about this piece of code, and pretend like you don't get that exception:

import java.util.ArrayList;


public class Marklength {

    void marklength4(ArrayList <String> themarklength){
        for(String n : themarklength){
            if(n.length() ==4){
                themarklength.add("****");
            }
        }
        System.out.println(themarklength);
    }
}

Ok, so what happens if your list just contains item.

You hit the line if(n.length() ==4){, which is true because you are looking at item, so you go execute its block.

Next you hit the line themarklength.add("****");. Your list now has the element **** at the end of it.

The loop continues, and you get the next item in the list, which happens to be the one you just added, ****.

The next line you hit is if(n.length() ==4){. This is true, so you execute its block. You go to the line themarklength.add("****");, and add **** to the end of the list.

Do we see a bad pattern here? Yes, yes we do.

The Java runtime environment also knows that this is bad, which is why it prevents something called Concurrent Modification. In your case, this means you cannot modify a list while you are iterating over it, which is what that for loop does.

My best guess as to what you are trying to do is something like this:

import java.util.ArrayList;


public class Marklength {

    ArrayList<String> marklength4(ArrayList <String> themarklength){
        ArrayList<String> markedStrings = new ArrayList<String>(themarklength.size());
        for(String n : themarklength){
            if(n.length() ==4){
                markedStrings.add("****");
            }
            markedStrings.add(n);
        }
        System.out.println(themarklength);
        return markedStrings;
    }
}

And then:

import java.util.ArrayList;


public class MarklengthTestDrive {
    public static void main(String[] args){

        ArrayList <String> words = new ArrayList<String>(); 

        words.add("Kane");
        words.add("Cane");
        words.add("Fame");
        words.add("Dame");
        words.add("Lame");  
        words.add("Same");

        Marklength ish = new Marklength();

        words = ish.marklength4(words);

    }
}
8
  • "Java compiler also knows" - No it doesn't Concurrent Modification is a runtime exception, not a compiler error, but the rest of the statement is true Mar 28, 2014 at 0:18
  • @MadProgrammer Oooh, very true, my bad. I'll fix that. Thanks. Mar 28, 2014 at 0:19
  • Okay, to earn that +1, what would you suggest as a fix? ;) Mar 28, 2014 at 0:23
  • @MadProgrammer I'm not actually sure what he wants, I just posted a comment asking him to clarify. From his question, he may want ["item"] to become ["****item"], ["****","item"], or even [["****"],"item"] (which isn't possible). Mar 28, 2014 at 0:25
  • 1
    I'll give a +1 for that. It's a nitpick and in no detracts from your answer, but I might suggest using ArrayList<String> markedStrings = new ArrayList<String>(themarklength.size()) as it can give better performance Mar 28, 2014 at 0:32
2

This...

if(n.length() ==4){
    themarklength.add("****");
}

Is simply trying to add "****" to the end of the list. This fails because the Iterator used by the for-each loop won't allow changes to occur to the underlying List while it's been iterated.

You could create a copy of the List first...

List<String> values = new ArrayList<String>(themarklength);

Or convert it to an array of String

String[] values = themarklength.toArray(new String[themarklength.size()]);

And uses these as you iteration points...

for (String value : values) {

Next, you need to be able to insert a new element into the ArrayList at a specific point. To do this, you will need to know the original index of the value you are working with...

if (value.length() == 4) {
    int index = themarklength.indexOf(value);

And then add a new value at the required location...

    themarklength.add(index, "****");

This will add the "****" at the index point, pushing all the other entries down

Updated

As has, correctly, been pointed out to me, the use of themarklength.indexOf(value) won't take into account the use case where the themarklength list contains two elements of the same value, which would return the wrong index.

I also wasn't focusing on performance as a major requirement for the providing a possible solution.

Updated...

As pointed out by JohnGarnder and AnthonyAccioly, you could use for-loop instead of a for-each which would allow you to dispense with the themarklength.indexOf(value)

This will remove the risk of duplicate values messing up the index location and improve the overall performance, as you don't need to create a second iterator...

// This assumes you're using the ArrayList as the copy...
for (int index = 0; index < themarklength.size(); index++) {
    String value = themarklength.get(index);
    if (value.length() == 4) {
        themarklength.add(index, "****");
        index++;

But which you use is up to you...

10
  • not sure where the downvote came from, yours is the same as the upvoted one (maybe post edit?) if you use a for loop instead of the foreach style one, you don't need the extra indexof. Mar 28, 2014 at 0:36
  • @JohnGardner That's true. You have to love the courage of people who downvote questions and answers without leaving a comment...how are people suppose to learn or have the chance to improve the quality of the post :P Mar 28, 2014 at 0:37
  • @JohnGardner I'm certainly not in it for the votes, as I helped the leading question to provide a possible answer ;) Mar 28, 2014 at 0:41
  • yeah, I know I was just pointing out the sometimes-insanity of downvotes :) Mar 28, 2014 at 0:43
  • @JohnGardner, I'm upvoting, I was the original downvoter but left for dinner. The reason for my initial downvote was that the indexOf(value) strategy was broken. It was wrong both in terms of performance - requiring an extra iterations every time that you find an element - and in terms correctness - if you have two equal strings if would insert **** twice before the first position. In order to to make it work you have to track the last index and use the indexOf(String str, int fromIndex) variant, which would still require partial new iterations. Mar 28, 2014 at 2:09
1

The problem is that in your method, you didn't modify each string in the arraylist, but only adds 4 stars to the list. So the correct way to do this is, you need to modify each element of the arraylist and replace the old string with the new one:

void marklength4(ArrayList<String> themarklength){
    int index = 0;
    for(String n : themarklength){
        if(n.length() ==4){
            n = "****" + n;
        }
        themarklength.set(index++, n);
    }
    System.out.println(themarklength);
}

If this is not what you want but you want to add a new string "**" before each element in the arraylist, then you can use listIterator method in the ArrayList to add new additional element before EACH string if the length is 4.

    ListIterator<String> it = themarklength.listIterator();
    while(it.hasNext()) {
        String name = it.next();
        if(name.length() == 4) {
            it.previous();
            it.add("****");
            it.next();
        }
    }

The difference is: ListIterator allows you to modify the list when iterating through it and also allows you to go backward in the list.

1
  • Yeah, that's what I thought the OP wanted to do as well, but I believe they want to add "****" BEFORE each matching element, not prefix it - could be wrong, but that's how I now read it :P Mar 28, 2014 at 0:22
1

I would use a ListIterator instead of a for each, listiterator.add likely do exactly what you want.

public void marklength4(List<String> themarklength){
    final ListIterator<String> lit = 
        themarklength.listIterator(themarklength.size());
    boolean shouldInsert = false;   
    while(lit.hasPrevious()) {
        if (shouldInsert) {
            lit.add("****");
            lit.previous();
            shouldInsert = false;
        }
        final String n = lit.previous();
        shouldInsert = (n.length() == 4);
    }
    if (shouldInsert) {
        lit.add("****");
    }
}

Working example

2
  • I thought this might break, and while I like the idea, it will add values "after" the last element, requiring to start messing about with previous, next...:P Mar 28, 2014 at 0:20
  • @MadProgrammer, acutally, before just means iterate in reverse. Check my example. Mar 28, 2014 at 1:41
0

Oh I remember this lovely error from the good old days. The problem is that your ArrayList isn't completely populated by the time the array element is to be accessed. Think of it, you create the object and then immediately start looping it. The object hence, has to populate itself with the values as the loop is going to be running.

The simple way to solve this is to pre-populate your ArrayList.

public class MarklengthTestDrive {
    public static void main(String[] args){

        ArrayList <String> words = new ArrayList<String>() {{ 

        words.add("Kane");
        words.add("Cane");
        words.add("Fame");
        words.add("Dame");
        words.add("Lame");  
        words.add("Same");
        }};
    }
}

Do tell me if that fixes it. You can also use a static initializer.

1
  • So I got "Cannot refer to a non-final variable words inside an inner class defined in a different method." Mar 28, 2014 at 0:17
0

make temporary arraylist, modify this list and copy its content at the end to the original list

    import java.util.ArrayList;

    public class MarkLength {
    void marklength4(ArrayList <String> themarklength){
        ArrayList<String> temp = new ArrayList<String>();
        for(String n : themarklength){
            if(n.length() ==4){
                temp.add(n);
                temp.add("****");
            }
        }
        themarklength.clear();
        themarklength.addAll(temp);
        System.out.println(themarklength);
    }
}

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.