Using Java 1.8, I created the following implementation to concat
two int arrays:
public static int[] concat(int[] array1, int[] array2) {
return IntStream.concat(Arrays.stream(array1), Arrays.stream(array2)).toArray();
}
The unit test I created to test this looks like this:
int[] numbers1 = new int[] {1, 4, 3, 10, 2, 11, 15, 8};
int[] numbers2 = new int[] {5, 6, 7, 9, 12, 13, 14};
@Test
public void concat() {
int[] concatNumbers = ArrayUtils.concat(numbers1, numbers2);
for (int i = 0; i < numbers1.length; i++) {
assert(concatNumbers[i] == numbers1[i]);
}
for (int i = numbers2.length + 1; i < numbers2.length; i++) {
assert(concatNumbers[i] - 1 == numbers1[i]);
}
}
Questions:
- Although my unit test works correctly, is there a better way to unit test this method (e.g. in terms of style, cleaner code, etc.)?
How could I change the actual implementation if I were to use varargs?
public static int[] concat(int[] ... args)
-
\$\begingroup\$ Thanks for editing Jamal... How did you add the syntax coloring for concat in the first sentence? \$\endgroup\$PacificNW_Lover– PacificNW_Lover2016年07月20日 05:56:37 +00:00Commented Jul 20, 2016 at 5:56
-
\$\begingroup\$ If you click the edit button, on any post you can see how they are formatted (and then discard the edit when you are done). Inline code formatting is done using the backtick (it's in the top left of my keyboard, but yours may vary). \$\endgroup\$forsvarir– forsvarir2016年07月20日 06:06:45 +00:00Commented Jul 20, 2016 at 6:06
4 Answers 4
Testing
concat
isn't a great name for a test, it doesn't tell me anything about what it is that the test does, it simply copies the name of the method being called. Something likeconcatShouldReturnConcatenationForTwoPopulatedArrays
might be a better descriptionPersonally, for a test this simple, I'd tend to define the expected results for the test within it rather than calculating it on the fly. This makes it less likely that you'll get a failure because your test is wrong. It also allows you to use the junit array comparison.
Assert.assertArrayEquals( expectedResult, result );
You've only got one test, I'd tend to add at least 3 more for:
concat([], numbers2) concat(numbers1, []) concat([], [])
You should add more test cases and use more descriptive names to define what is the purpose of each test.
You should be careful with your tests, if you have errors in your tests you could get unexpected results. In your code, you are not testing your input as much as you expect. There is a bug in the test at your second loop:
// Initial value of i is greater than the breaking condition
for (int i = numbers2.length + 1; i < numbers2.length; i++) {
// Dead code, you are not testing this part of the input
assert(concatNumbers[i] - 1 == numbers1[i]);
}
You second loop could be:
for (int i = 0; i < numbers2.length; i++) {
assert(concatNumbers[numbers1.length + i] == numbers2[i]);
}
As a first part of the test I would recommend to verify
concatNumbers.length == numbers1.length + numbers2.length
And the second for
seems a total mess to me.
Did you run your test at least once?
The Arrays class can make things more elegant.
public void concatNumbers() {
// Make the exptected result:
// - Here by a reference implementation, but a manual { ... } would do too.
int[] expectedNumbers = Arrays.copyOf(numbers1, numbers1.length + numbers2.length);
System.arraycopy(numbers2, 0, expectedNumbers, numbers1.length, numbers2.length);
int[] concatNumbers = ArrayUtils.concat(numbers1, numbers2);
assertTrue(Arrays.equals(concatNumbers, expectedNumbers));
}
Explore related questions
See similar questions with these tags.