incubator-cassandra-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Edward Ribeiro <edward.ribe...@gmail.com>
Subject Column is immutable ... no, it's not
Date Wed, 08 Jul 2009 01:06:29 GMT
Column's javadoc says that it's an immutable class, but look at the
following constructor:

    Column(String name, byte[] value, long timestamp, boolean isDeleted)
    {
        assert name != null;
        assert value != null;
        this.name = name;
        this.value = value;
        this.timestamp = timestamp;
        isMarkedForDelete = isDeleted;
    }

Everything is OK, right? If you think so, then run the following
method as a (junit) test :

	public void testColumn() {
		byte[] buf = "hello".getBytes();
		Column c = new Column("test", buf);
		assertEquals("hello", new String(c.value()));
		buf[1] = 'x';
		assertEquals("hello", new String(c.value()));
	}


Surprise! It fails at the second assertEquals.

Well, the byte array is a reference to an object that is not under
control of the Column object. If an object, that is part of the state
of Column, can be changed then Column is not immutable by any means.

If you think the little test method is not reallistic and nobody will
ever do this then you are assuming that programmers don't create bugs,
ever. In addition, the javadoc says something that simply is not true.
Therefore, you have two options: change the documentation or change
the constructor.

Let's see what needs to be done to turn Column into an 100% immutable class:

First, I create a utility method that just copies the content of the
byte array to a internal byte array inside Column class. This is what
we call a defensive copy. The other arguments are immutable (String)
or primitive types so they don't need defensive copy. But the byte
array is an achilles' heel lurking around.

    private byte[] copyArray(byte[] src)
    {
    	byte[] newArray = new byte[src.length];
    	System.arraycopy(src, 0, newArray, 0, newArray.length);
    	return newArray;
    }

Then, let's make a little change to the constructor (I ommited parts
of the code):

    Column(String name, byte[] value, long timestamp, boolean isDeleted)
    {
        [....]
        this.value = copyArray(value);
        [....]
    }

Run the tests again it will display a beautiful green bar. :) Now,
Column is a immutable class by excellence. :)

Edward
PS: Effective Java by Joshua Bloch, 2nd edition, is the book that have
this and many other tricks.

Mime
View raw message