7
\$\begingroup\$

I'm saving string values in an array one at a time so I cannot initialize the array with the values all at once. After I'm done with the array I need to pass it to another class that is expected a String array.

Normally I would use a List but the end result is expecting an Array so I stuck with the Array. I'm having second thoughts though because maybe it would have better performance if I used a List then when I'm finished filling up the List I could use List.toArray and get the Array that way? Here is majority of the Array code I wrote that takes one argument at a time and adds it to the Array,

public String[] addArgument(String[] arguments, String arg) {

    String[] temp = new String[arguments.length + 1];

    for(int i=0; i<arguments.length; i++)
        temp[i] = arguments[i];

    temp[arguments.length] = arg;

    arguments = temp;

    temp = null;

    return arguments;
}
\$\endgroup\$
  • \$\begingroup\$ You should add more context to your question, show some of the code that calls addArgument(...) \$\endgroup\$ – rolfl Jul 22 '14 at 16:57
  • \$\begingroup\$ Why it's a large program? You see the function declaration and that's all you need. \$\endgroup\$ – Kyle Bridenstine Jul 22 '14 at 16:58
  • \$\begingroup\$ Because your function is wrong, and you should have it declared differently .... ;-) \$\endgroup\$ – rolfl Jul 22 '14 at 16:59
  • \$\begingroup\$ The function works what's wrong about it? \$\endgroup\$ – Kyle Bridenstine Jul 22 '14 at 17:00
  • 1
    \$\begingroup\$ Your question is basically: How do I treat an array like a list, yet you have only shown us a really poor-performance method that takes one array, creates another one that's 1 bigger, and copies the first array content to the second, and adds a new string. That is only a small (and slow) part of the bigger problem you are trying to solve. The answers you have so far show a convenient, and different solution, which is how to use a list, as a list, which is not your stated problem \$\endgroup\$ – rolfl Jul 22 '14 at 17:22
8
\$\begingroup\$

You should use the List#toArray method, you want to use the generic method, so not the one returning Object[], for this to work you need to supply an array of the correct size, such that it can store the data there.

List<String> list = ...;
String[] = list.toArray(new String[list.size()]);

Note that the array need not be the same size, there are two cases to consider:

  • If array.length < list.size(), then it will create a new array and return that one.
  • Else, it will put the data in the input array.
\$\endgroup\$
5
\$\begingroup\$

Two small comments:

Spacing:

You put every single instruction on a separate line with additional newlines around. IMO that is completely overkill and reduces the readability of code. It's harder to subdivide code into logical sections when you read.

But aside from that, you seem to mostly ignore the Spacebar, which cramps your operators together and makes them hard to find in between all the letters.

String[] temp = new String[arguments.length + 1];

for(int i=0; i<arguments.length; i++)
    temp[i] = arguments[i];

temp[arguments.length] = arg;

arguments = temp;

return arguments;

compare your code [^] with how I would have written the same instructions and decide for yourself which is better:

String[] temp = new String[arguments.length +1];

for (int i = 0; i < arguments.length; i++) {
    temp[i] = arguments[i];
}
temp[arguments.length] = arg;

arguments = temp;
return arguments;

Clearing:

In there is absolutely no need to purge references, the garbage collector will do that for you:

temp = null;

is utterly useless and should be removed.

\$\endgroup\$
  • 1
    \$\begingroup\$ Simply return temp after the arg is set and avoid the reassignment as well. \$\endgroup\$ – Robin Jul 23 '14 at 18:47
4
\$\begingroup\$

you can make it more efficient with System.arraycopy

public static String[] addArgument(String[] arguments, String arg) {
    String[] temp = new String[arguments.length + 1];
    System.arraycopy(arguments, 0, temp, 0, arguments.length);
    temp[arguments.length] = arg;
    return temp;
}

(also removing some unnecessary assignments). Moving it to a list, doing your modifcations and back can make it more efficient if doing a lot of operations on it since this is expensive if the arguments array is of any size. Best if only consider arrays for more immutable things and use collection's data structures for anything more complex unless theres a really good reason.

\$\endgroup\$
1
\$\begingroup\$

Use a list for building your array, then convert your list into an array before you return it from your function. =)

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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