3
\$\begingroup\$

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:

  1. 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.)?
  2. How could I change the actual implementation if I were to use varargs?

    public static int[] concat(int[] ... args) 
    
200_success
146k22 gold badges190 silver badges479 bronze badges
asked Jul 20, 2016 at 5:15
\$\endgroup\$
2
  • \$\begingroup\$ Thanks for editing Jamal... How did you add the syntax coloring for concat in the first sentence? \$\endgroup\$ Commented 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\$ Commented Jul 20, 2016 at 6:06

4 Answers 4

4
\$\begingroup\$

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 like concatShouldReturnConcatenationForTwoPopulatedArrays might be a better description
  • Personally, 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([], [])
    
answered Jul 20, 2016 at 8:10
\$\endgroup\$
2
\$\begingroup\$

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]);
}
answered Jul 21, 2016 at 7:37
\$\endgroup\$
0
\$\begingroup\$

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?

answered Jul 21, 2016 at 8:38
\$\endgroup\$
0
\$\begingroup\$

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));
}
answered Jul 21, 2016 at 11:39
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

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

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.