0

I have refered : Security - Array is stored directly.

My code is as 

public IndexBlockAdapter(String[] itemStr) {
    if(itemStr == null) { 
        this.itemStr = new String[0]; 
    } else { 
        this.itemStr = Arrays.copyOf(itemStr, itemStr.length); 
    }
}

But Sonar still picks it up and complains about "Array is stored directly", despite making a copy. I am very confused.

Any help is appreciated!

Community
  • 1
  • 1
Jason Xu
  • 845
  • 8
  • 22

2 Answers2

3
Arrays.copyOf does a shallow copy.

It just copies the references and not the actual values. The following code will print all true which proves the fact

String [] str1 = {"1","2","3"};

    String [] str2 = Arrays.copyOf(str1, str1.length);
    for (int i=0;i<str1.length;i++) {
        System.out.println(str1[i] == str2[i]);

    }

Instead, if you use the following code, you will do a deep copy, and you should be good

String [] str3 = new String[str1.length];
for (int i=0;i<str1.length;i++) {
    str3[i] = new String(str1[i]);
}
for (int i=0;i<str1.length;i++) {
    System.out.println(str1[i] == str3[i]);
}
Hirak
  • 3,601
  • 1
  • 22
  • 33
  • Although this might be the reason, why the copy mechanism is complained about, a shallow copy is still enough. Keep in mind: _String is immutable_. – Seelenvirtuose May 05 '14 at 11:53
  • Thanks！I think using `System.arrayCopy()` or `Object.clone()` can also do a deep copy, which is more simple. @Hirak – Jason Xu May 30 '14 at 07:20
  • Finally, I find the real reason is that name of the parameter can not be the same as the instance variable. When I changed the parameter name to `itemArr`, the warning disappeared . – Jason Xu Jun 13 '14 at 02:12
0

This should work for you

 public IndexBlockAdapter(String[] newItemStr) {
 if(newItemStr == null) { 
    this.itemStr = new String[0]; 
 } else { 
    this.itemStr = Arrays.copyOf(newItemStr, newItemStr.length); 
 }
}
Some Java Guy
  • 4,992
  • 19
  • 71
  • 108