add and get String Array into and from Arraylist

Question

I have this code that reads from XML file. It gets five strings (groupId, groupType, filePath, author, and lineNo), and it first saves them in a String Array. Then, the String Array is saved in an ArrayList. Finally, the last "for" displays the content of the ArrayList.

The problem that when I want to display the content, I get just the last added string array. The following is the code and the output. Can anyone figure out what is the problem?

ArrayList<String[]> developerTypes = new ArrayList<String[]>();
String[] developerInfo = {null, null, null, null, null};
String[] developerInfoR = {null, null, null, null, null};

String groupId;
String groupType;
String filePath;
String author;
String lineNo;


SAXBuilder builder = new SAXBuilder();
Document doc = (Document) builder.build("A.xml");
Element clones = doc.getRootElement();

// Loop of clones' children (clone_group)
List<Element> parentElements = clones.getChildren();
for(Element parentElement:parentElements){


    // Loop of clone_group's children (clone_fragment)
    List<Element> elements = parentElement.getChildren();
    for(Element element:elements){

        // Loop of clone_fragment's children (blameInfo)
        List<Element> childelements = element.getChildren();
        for(Element childElement:childelements){

            groupId = parentElement.getAttributeValue("groupid");
            groupType = parentElement.getAttributeValue("type");
            filePath = element.getAttributeValue("file");
            author = childElement.getAttributeValue("author");
            lineNo = childElement.getAttributeValue("lineNo");
            //System.out.print(groupId + " - ");
            //System.out.print(groupType + " - ");
            //System.out.print(file + " - ");
            //System.out.println(author);
            developerInfo[0] = groupId;
            developerInfo[1] = groupType;
            developerInfo[2] = filePath.substring(1, filePath.lastIndexOf("."));;
            developerInfo[3] = author;
            developerInfo[4] = lineNo;
            developerTypes.add(developerInfo);

        }// for (blameInfo)    
    }// for (clone_fragment)
}// for (clone_group)

// Display the content of the Arraylist    
for(int i = 0; i< developerTypes.size(); ++i){

    developerInfoR = developerTypes.get(i);

    for(int j = 0; j< developerInfoR.length; ++j){

        System.out.print(developerInfoR[j] + " ");

    }
    System.out.print("\n");

}

The Output:

309 Type-3 builtin/update-index.c Jonathan Nieder 704 
309 Type-3 builtin/update-index.c Jonathan Nieder 704 
309 Type-3 builtin/update-index.c Jonathan Nieder 704 
309 Type-3 builtin/update-index.c Jonathan Nieder 704 
309 Type-3 builtin/update-index.c Jonathan Nieder 704 
309 Type-3 builtin/update-index.c Jonathan Nieder 704 
309 Type-3 builtin/update-index.c Jonathan Nieder 704 
309 Type-3 builtin/update-index.c Jonathan Nieder 704 
309 Type-3 builtin/update-index.c Jonathan Nieder 704 
309 Type-3 builtin/update-index.c Jonathan Nieder 704 
309 Type-3 builtin/update-index.c Jonathan Nieder 704 
309 Type-3 builtin/update-index.c Jonathan Nieder 704 
...

Seems to me you need to know how to handle a good IDE with debugger, the error will become quickly apparent when you "step through" code like this. — Maarten Bodewes
– Maarten Bodewes, Commented Jun 27, 2012 at 20:10

Jon Skeet · Accepted Answer · 2012-06-27 20:09:50Z

3

The problem that when I want to display the content, I get just the last added string array.

No, you find that you've got many references to the same string array... because that's what you've added. You've only got one string array object; developerInfo is just a reference to that array. When you call developerTypes.add(developerInfo) that's copying the reference into the ArrayList, so you've got the same reference lots of times.

You should pull the declaration and instantiation of developerInfo into the loop:

String[] developerInfo = {
    groupId,
    groupType,
    filePath.substring(1, filePath.lastIndexOf(".")),
    author,
    lineNo
};
developerTypes.add(developerInfo);

Likewise your code would be cleaner if you didn't declare developerInfoR until you used it:

for(int i = 0; i< developerTypes.size(); ++i){
    String[] developerInfoR = developerTypes.get(i);
    for(int j = 0; j< developerInfoR.length; ++j){
        System.out.print(developerInfoR[j] + " ");
    }
    System.out.print("\n");
}

Or even better, use the enhanced for loop:

for (String[] developerInfoR : developerTypes) {
    for (String info : developerInfoR) {
        System.out.print(info + " ");
    }
    System.out.print("\n");
}

In general, you should declare local variables with the smallest scope you can get away with, as late as possible, ideally assigning a value at the point of declaration. Declaring all the variables at the top of the method really hurts readability.

answered Jun 27, 2012 at 20:09

Jon Skeet

1.5m893 gold badges9.3k silver badges9.3k bronze badges

Sign up to request clarification or add additional context in comments.

2 Comments

Charles Over a year ago

It also makes it really easy to identify C programmers.

Marko Topolnik Over a year ago

@Charles Not any C programmers, mind you, but K&R-style C programmers :)

Marko Topolnik · Accepted Answer · 2012-06-27 20:39:14Z

You have only one instance of the developerInfo array that you keep adding over and over in the loop. You need to make a new array in each loop step.

You seem to have some misunderstanding of the semantics of the contsructs you are using. I'm deducing that from the way you are initializing developerInfoR up front, but never using that value. An array variable only contains a reference to the array, so every time you assign to it, the array referenced before gets forgotten and thrown away. So, don't initialize developerInfo up front. In fact, you don't even need to declare it until the place you are using it.

There is also a simpler way to print an array's contents: just call System.out.println(Arrays.toString(developerInfoR)). That way you don't even need the inner loop in the printing code.

You don't need any of the variables groupId, groupType, filePath, author, lineNo. Just write for example developerInfo[0] = parentElement.getAttributeValue("groupid"); and similar. That makes for more obvious code. But it will probably make sense to use a full object instead of a String[]. There are many fields and the meaning of each gets behind the integer index.

All in all, taking these suggestions plus a few additional ones, your code can be rewritten to this:

final String path = filePath.substring(1, filePath.lastIndexOf("."));
final Document doc = (Document) new SAXBuilder().build("A.xml");
final List<DeveloperInfo> developers = new ArrayList<String[]>();
for (Element parentElement : doc.getRootElement().getChildren())
  for (Element element : parentElement.getChildren())
    for (Element childElement : element.getChildren())
      developers.add(new DeveloperInfo(
          parentElement.getAttributeValue("groupid"),
          parentElement.getAttributeValue("type"),
          element.getAttributeValue("file"),
          path,
          childElement.getAttributeValue("author"),
          childElement.getAttributeValue("lineNo"),
      ));
for (DeveloperInfo d : developerTypes) System.out.println(d);

The DeveloperInfo class:

class DeveloperInfo {
  public final String groupId, groupType, filePath, author, lineNo;
  public DeveloperInfo(
      String groupId, String groupType, String filePath, 
      String author, String lineNo)
  {
    this.groupId = groupId; this.groupType = groupType; this.filePath = filePath;
    this.author = author; this.lineNo = lineNo;
  }
  public String toString() {
    return "DeveloperInfo [groupId=" + groupId + ", groupType=" + groupType + 
      ", filePath=" + filePath + ", author=" + author + ", lineNo=" + lineNo + "]";
  }

Rob Wagner · Accepted Answer · 2012-06-27 20:10:22Z

0

You are referencing the same array every time you add it to the ArrayList.

Instead of declaring this String[] developerInfo = {null, null, null, null, null};

at the beginning. Create a new array everytime you loop.

String[] developerInfo = new String[5];

answered Jun 27, 2012 at 20:10

Rob Wagner

4,46117 silver badges24 bronze badges

Collectives™ on Stack Overflow

add and get String Array into and from Arraylist

3 Answers 3

2 Comments

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

2 Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related