Skip to content

Every day is a school day, but sometimes you’ve moved down a class.

February 17, 2010

Another day, another chance to be a complete computard. Following on from my previous Java test, a friend gave me a similar challenge. How would you implement the following method?

	void swapArrays(byte[] a1, byte[] a2) {
		// your code here
	}

It’s a general rule that I’ll only answer questions after at least my second cup of tea of the morning, and my no-cup (and no-computer) answer shows why…

	void swapArrays(byte[] a1, byte[] a2) {
		byte[] tmp = a1;
		a1 = a2;
		a2 = tmp;
	}

Of course, it doesn’t work. In my defence, I knew I was doing something wrong, I just couldn’t put my finger on it. Sure, it wouldn’t work with bytes because bytes are primitives and primitives are passed-by-value, but arrays are objects, and objects are passed-by-reference, even if they’re arrays of primitives, right?

What I’d forgotten is that pass-by-reference is a lie; everything in Java is passed-by-value, it’s just a question of whether you’re passing a copy of a primitive, or a copy of a reference.

So, what is the right answer? I don’t know. This is the best I’ve come up with so far…

	void copyArrayContents(final byte[] source, byte[] destination) {
		if (destination.length < source.length) {
			throw new IllegalArgumentException(
					"Destination length insufficient to hold source");
		}
		// what if destination is longer than source?
		for (int i = 0; i < source.length; i++) {
			destination[i] = source[i];
		}
	}

	void swapArrays(byte[] a1, byte[] a2) {
		if (a1.length != a2.length) {
			throw new IllegalArgumentException(
					"Cannot swap arrays of different lengths");
		}

		byte[] tmp = new byte[a1.length];
		copyArrayContents(a1, tmp);
		copyArrayContents(a2, a1);
		copyArrayContents(tmp, a2);
	}

	@Test
	public void swapArraysTest() {
		byte[] uno = new byte[] { 1 };
		byte[] due = new byte[] { 2 };

		Assert.assertEquals(1, uno[0]);
		Assert.assertEquals(2, due[0]);

		swapArrays(uno, due);

		Assert.assertEquals(2, uno[0]);
		Assert.assertEquals(1, due[0]);
	}

What do you think? Here’s a random list of points that go through my head when I look at this code:

  • We could save some stack space by using a temporary element instead of a whole temporary array, but that’s lipstick-on-a-pig stuff.
  • Arrays are horrible.
  • Primitives are worse.
  • What should copyArrayContents do if the destination array is longer than the source? Bearing in mind that we could reasonably expect (and indeed should encourage) developers to reuse our method whenever they need to copy the contents of one array to another, and not necessarily access it through swapArrays.
  • [How] Can we swap arrays of differing length?
  • What ever happened to OO? Seriously, assuming that swapping arrays even makes sense in your application, it should be a method on your object. And by makes sense in your application I mean makes sense to the next developer who works on it. Which it won’t. So don’t do it.

I don’t know if I’m just narked at making such a hash of my first answer (likely) but it really annoys me when the “right” answer involves remembering some obscure corner-case of the language that should never have allowed in the first place, and the right answer is WTF are you doing writing such a retarded method function in the first place?!

Advertisements
5 Comments leave one →
  1. February 18, 2010 11:46 am

    Here’s an implementation of your first attempt that works:

        void swapArrays(byte[] a1, byte[] a2) {
            byte[] temp = java.util.Arrays.copyOf(a1, a1.length);
            System.arraycopy(a2, 0, a1, 0, a2.length);
            System.arraycopy(temp, 0, a2, 0, temp.length);
        }
    

    Although I find the following slightly more readable (but uses more memory I suppose):

        void swapArrays2(byte[] a1, byte[] a2) {
            byte[] left = Arrays.copyOf(a1, a1.length);
            byte[] right = Arrays.copyOf(a2, a2.length);
            System.arraycopy(right, 0, a1, 0, right.length);
            System.arraycopy(left, 0, a2, 0, left.length);
        }
    

    arraycopy will throw ArrayIndexOutOfBounds if there is a length mismatch.

    I have no idea why arraycopy is in System. Horrible method anyway. I say horrible because it mutates the given objects rather than returning something (like the methods in java.util.Arrays). But hey, that was the spec of the desired method for this puzzle.

    As you already pointed out, arrays and primitives suck. The real problem is that someone wanted this method. I’d personally implement it using collections and force them to convert their arrays if they want to use it. (they could use Arrays.asList(T… a))

    (As there’s no preview function for commenters, I can only assume this will render as unreadable garbage. Well tough, I’m not sorry about that, fix it.)

  2. February 18, 2010 11:59 am

    Oh, here’s a version that would truncate or pad with zeroes rather than throw an exception if there was an array length mismatch:

         @Test
        public void testMismatchedArraySizes() {
            byte[] uno = new byte[] { 1, 1 };
            byte[] due = new byte[] { 2 };
    
            swapArraysPadAndTrunc(uno, due);
    
            Assert.assertArrayEquals(new byte[] { 2, 0 }, uno);
            Assert.assertArrayEquals(new byte[] { 1 }, due);
        }
    
        void swapArraysPadAndTrunc(byte[] a1, byte[] a2) {
            byte[] left = Arrays.copyOf(a1, a2.length);
            byte[] right = Arrays.copyOf(a2, a1.length);
            System.arraycopy(right, 0, a1, 0, right.length);
            System.arraycopy(left, 0, a2, 0, left.length);
        }
    
  3. February 18, 2010 12:00 pm

    It’s all horrible and makes me feel sick and dirty. Where’s my lovely objects?

    Set the next puzzle on something nice and OOey.

  4. February 18, 2010 12:01 pm

    So what (I assume that) you’re saying (like I bother to read all of your comments), among other things, is it isn’t possible for swapArrays to swap array of different length. Which makes sense to me. But then my first answer made sense to me…

  5. February 18, 2010 1:10 pm

    Yeah, the size of an array is fixed forever and can’t be changed (that’s why we use Lists instead).

    The client could use the contents of your method if he wanted to swap the references but you can’t do that in a method. That’s because the method was passed a copy of the reference the client has. All you can do is change the state of object that reference points to, you can’t change where his reference points.

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s

%d bloggers like this: