HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Circular buffer implementation for buffering TCP socket messages

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
tcpbufferingcircularmessagesforimplementationsocketbuffer

Problem

I have written my own for practice and also some tweaks for my specific requirement. There is an insert which can copy a range (well array copy really) which I am hoping is more efficient than individual element insert.

Can anyone make any comments specifically as regards efficiency and code elegance. I need to use this with Java 1.6 and am getting a warning on the array = newArray(size); line in the constructor. But that is the only issue I am currently aware of.

```
import java.util.Arrays; //copyOf

public class CircularBuffer {
private T[] array; //storage
private int put_idx, get_idx, count; //get/put & internal num elements counter
private final int size; //size of internal array storage

public CircularBuffer(int size) {
get_idx = 0;
put_idx = 0;
count = 0;
this.size = size;

//Type safety: A generic array of T is created for a varargs parameter - investigate how to remove warning
array = newArray(size);
}

//@SafeVarargs - ok in java 1.7
static T[] newArray(int length, T... array)
{
return Arrays.copyOf(array, length);
}

void reset(){
get_idx=put_idx=count=0;
}

public boolean empty() { return count == 0; }

public boolean full() { return count == size; }

int get_count() { return count; }

//inserts a single new element
public void insert(T element)
{
if(full())
throw new ArrayIndexOutOfBoundsException("buffer full");

array[put_idx++] = element;
put_idx %= size;
++count;
}

//insert array of specified length
public void insert(T[] thearray) {
if(thearray.length + count > size)
throw new ArrayIndexOutOfBoundsException("insufficient space to insert array");

//first copy to end - contiguous array copy
int remaining = size - put_idx;
if(remaining >= thearray.length) {
System.arraycopy(thearray,

Solution

A few minor notes:

-
Comments like the following are unnecessary:

private T[] array; // storage


You could call the field as storage and remove the comment.

-
A "solution" for the warning in the constructor:

@SuppressWarnings("unchecked")
private T[] newArray(int length) {
    return (T[]) new Object[length];
}


Reference: Java how to: Generic Array creation

-
camelCase variable and method names are more common in Java than underscored ones. (Code Conventions for the Java Programming Language, 9 - Naming Conventions)

-
From Code Complete, 2nd Edition, p761:


Use only one data declaration per line


[...]


It’s easier to modify declarations because each declaration is self-contained.


[...]


It’s easier to find specific variables because you can scan a single
column rather than reading each line. It’s easier to find and fix
syntax errors because the line number the compiler gives you has
only one declaration on it.

-
Setting default integer field values to zero is unnecessary in the constructor since zero is the default int value:

getIdx = 0;
putIdx = 0;
count = 0;


-
Does it make sense to create a CircularBuffer instance with zero size? If not check it and throw an IllegalArgumentException in the constructor.

-
The second insert method might be a premature optimization.

-
I'd throw IllegalStateException instead of ArrayIndexOutOfBoundsException. The clients of the class should not know that it uses an array (or anything else) for storage.

-
Here are a few unit tests:

```
@Test(expected = IllegalArgumentException.class)
public void testZeroSize() {
new CircularBuffer(0);
}

@Test
public void testIsEmptyOnNewBuffer() {
final CircularBuffer buffer = new CircularBuffer(3);
assertTrue(buffer.isEmpty());
}

@Test
public void testIsFullOnNewBuffer() {
final CircularBuffer buffer = new CircularBuffer(3);
assertFalse(buffer.isFull());
}

@Test
public void testIsEmpty() {
final CircularBuffer buffer = new CircularBuffer(3);
buffer.insert("a");
assertFalse(buffer.isEmpty());
}

@Test
public void testIsFull() {
final CircularBuffer buffer = new CircularBuffer(3);
buffer.insert("a");
buffer.insert("b");
buffer.insert("c");
assertTrue(buffer.isFull());
}

@Test
public void testIsEmptyAfterReset() {
final CircularBuffer buffer = new CircularBuffer(3);
buffer.insert("a");
buffer.reset();
assertTrue(buffer.isEmpty());
}

@Test
public void testIsFullAfterReset() {
final CircularBuffer buffer = new CircularBuffer(3);
buffer.insert("a");
buffer.insert("b");
buffer.insert("c");
buffer.reset();
assertFalse(buffer.isFull());
}

@Test
public void testIsEmptyOnUsedBuffer() {
final CircularBuffer buffer = new CircularBuffer(3);
buffer.insert("a");
buffer.insert("b");
buffer.insert("c");

assertFalse("isEmpty 1", buffer.isEmpty());
buffer.get();
assertFalse("isEmpty 2", buffer.isEmpty());
buffer.get();
assertFalse("isEmpty 3", buffer.isEmpty());
buffer.get();
assertTrue("isEmpty 4", buffer.isEmpty());
}

@Test
public void testIsFullOnUsedBuffer() {
final CircularBuffer buffer = new CircularBuffer(3);
buffer.insert("a");
buffer.insert("b");
buffer.insert("c");
buffer.get();

assertFalse("isFull 1", buffer.isFull());
buffer.get();
assertFalse("isFull 2", buffer.isFull());
buffer.get();
assertFalse("isFull 3", buffer.isFull());
}

@Test
public void testInsert() {
final CircularBuffer buffer = new CircularBuffer(3);
buffer.insert("a");
buffer.insert("b");
buffer.insert("c");
}

@Test(expected = IllegalStateException.class)
public void testInsertFull() {
final CircularBuffer buffer = new CircularBuffer(3);
buffer.insert("a");
buffer.insert("b");
buffer.insert("c");
buffer.insert("d");
}

@Test(expected = IllegalStateException.class)
public void testGetOnEmptyBuffer() {
final CircularBuffer buffer = new CircularBuffer(3);
buffer.get();
}

@Test(expected = IllegalStateException.class)
public void testGetOnEmptyUsedBuffer() {
final CircularBuffer buffer = new CircularBuffer(3);
buffer.insert("a");
buffer.get();

buffer.get();
}

@Test
public void testGet() {
final CircularBuffer buffer = new CircularBuffer(3);
buffer.insert("a");
assertEquals("a", buffer.get());
buffer.insert("b");
buffer.insert("c");
}

@Test
public void testDelayedGet() {
final CircularBuffer buffer = new CircularBuffer(3);
buffer.insert("a");
buffer.insert("b");
buffer.insert("c");
assertEquals("a", buffer.get());
}

@Test
public void testInsertAndGetTwice() {
final CircularBuffer buffer = new CircularBuffer(3);
buffer.insert("a");
buffer.insert("b");
buffer.insert("c");
assertEquals("a", buffer.get());
assertEquals("b", buffer.get());
assertEquals("c", buffer.get());

buffer.inser

Code Snippets

private T[] array; // storage
@SuppressWarnings("unchecked")
private T[] newArray(int length) {
    return (T[]) new Object[length];
}
getIdx = 0;
putIdx = 0;
count = 0;
@Test(expected = IllegalArgumentException.class)
public void testZeroSize() {
    new CircularBuffer<String>(0);
}

@Test
public void testIsEmptyOnNewBuffer() {
    final CircularBuffer<String> buffer = new CircularBuffer<String>(3);
    assertTrue(buffer.isEmpty());
}

@Test
public void testIsFullOnNewBuffer() {
    final CircularBuffer<String> buffer = new CircularBuffer<String>(3);
    assertFalse(buffer.isFull());
}

@Test
public void testIsEmpty() {
    final CircularBuffer<String> buffer = new CircularBuffer<String>(3);
    buffer.insert("a");
    assertFalse(buffer.isEmpty());
}

@Test
public void testIsFull() {
    final CircularBuffer<String> buffer = new CircularBuffer<String>(3);
    buffer.insert("a");
    buffer.insert("b");
    buffer.insert("c");
    assertTrue(buffer.isFull());
}

@Test
public void testIsEmptyAfterReset() {
    final CircularBuffer<String> buffer = new CircularBuffer<String>(3);
    buffer.insert("a");
    buffer.reset();
    assertTrue(buffer.isEmpty());
}

@Test
public void testIsFullAfterReset() {
    final CircularBuffer<String> buffer = new CircularBuffer<String>(3);
    buffer.insert("a");
    buffer.insert("b");
    buffer.insert("c");
    buffer.reset();
    assertFalse(buffer.isFull());
}

@Test
public void testIsEmptyOnUsedBuffer() {
    final CircularBuffer<String> buffer = new CircularBuffer<String>(3);
    buffer.insert("a");
    buffer.insert("b");
    buffer.insert("c");

    assertFalse("isEmpty 1", buffer.isEmpty());
    buffer.get();
    assertFalse("isEmpty 2", buffer.isEmpty());
    buffer.get();
    assertFalse("isEmpty 3", buffer.isEmpty());
    buffer.get();
    assertTrue("isEmpty 4", buffer.isEmpty());
}

@Test
public void testIsFullOnUsedBuffer() {
    final CircularBuffer<String> buffer = new CircularBuffer<String>(3);
    buffer.insert("a");
    buffer.insert("b");
    buffer.insert("c");
    buffer.get();

    assertFalse("isFull 1", buffer.isFull());
    buffer.get();
    assertFalse("isFull 2", buffer.isFull());
    buffer.get();
    assertFalse("isFull 3", buffer.isFull());
}

@Test
public void testInsert() {
    final CircularBuffer<String> buffer = new CircularBuffer<String>(3);
    buffer.insert("a");
    buffer.insert("b");
    buffer.insert("c");
}

@Test(expected = IllegalStateException.class)
public void testInsertFull() {
    final CircularBuffer<String> buffer = new CircularBuffer<String>(3);
    buffer.insert("a");
    buffer.insert("b");
    buffer.insert("c");
    buffer.insert("d");
}

@Test(expected = IllegalStateException.class)
public void testGetOnEmptyBuffer() {
    final CircularBuffer<String> buffer = new CircularBuffer<String>(3);
    buffer.get();
}

@Test(expected = IllegalStateException.class)
public void testGetOnEmptyUsedBuffer() {
    final CircularBuffer<String> buffer = new CircularBuffer<String>(3);
    buffer.insert("a");
    buffer.get();

    buffer.get();
}

@Test
public void testGet() {
    final CircularBuffer<String> buffer = new CircularBuffer<S

Context

StackExchange Code Review Q#22826, answer score: 2

Revisions (0)

No revisions yet.