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

A buffer that holds and provides the latest n items

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

Problem

I have developed the following code to hold n latest items it has received, and when asked for provide these n latest items.
The interface is:

public interface ICircularBuffer {
        void Put(T item);  // put an item
        T[] Read(); // provides the last "n" requests
}


The idea is to build a buffer that will hold "n" latest items that have been put and return those items when asked for.

For example:

var buf = new ConcurrentBuffer(3); // creates a buffer of size = 3
buf.Put(15);
buf.Put(10);
buf.Put(20);

int[] arr = buf.Read() // returns - 20,10,15

buf.Put(25);
int[] arr = buf.Read() // returns - 25,20,10


The implementation uses "locks" and array to represent the circular buffer:

public class ConcuurentCircularBuffer : ICircularBuffer {

        private T[] buffer;
        private int last = 0;
        private int sz;
        private object lockObject = new object();

        public ConcuurentCircularBuffer(int sz) {
            // array index starts at 1
            this.sz = sz;
            buffer = new T[sz + 1];
        }

        public void Put(T item) {
            lock (lockObject) {
                last++;
                last = last > sz ? 1 : last;
                buffer[last] = item;
            }
        }

        public T[] Read() {
            T[] arr = new T[sz];

            lock (lockObject) {
                int iterator = 0;
                for (int read = 0; read < sz; read++) {
                    int index = last - iterator;
                    index = index <= 0 ? (sz + index) : index;
                    if (buffer[index] != null) {
                        arr[iterator] = buffer[index];
                    } else {
                        break;
                    }
                    iterator++;
                }
            }
            return arr;
        }
    }


The unit tests I am using to check the performance are as follows:

```
[TestMethod()]
public void TestParallelPut() {

Solution

Naming

  • There is one 'u' in ConcuurentCircularBuffer that wants to be a 'r' --> ConcurrentCircularBuffer



  • There is no need to abbreviate name like size (sz --> size)



  • I would call last to something more descriptive like lastIndex.



Code Style

  • sz, buffer and lockObject should be read-only.



  • There is no need to skip the first array element, just start with 0.



  • iterator has always the same value as read. Therfore you can drop it.



Behavior

  • The array returned by Read has always the same size as the buffer, independend of the actual number of items. To avoid that, consider to change the type of arr to List.



  • If one of the buffer's elements is null, you assume, that the end of the buffer is reached. Therefore: circularBuffer.Put(null) would clear the buffer. That is an unexpected behavior. Suggestion: Don't allow to put null values to the buffer (throw ArgumentNullException) and add a method Clear if needed.



  • When using a primitive type like int for T, the Read method returns an array with the size of the buffer with the default value, even if no item was added. That is also a little bit strange... I suppose you need something like a count property that stores the number of added items.

Context

StackExchange Code Review Q#135562, answer score: 11

Revisions (0)

No revisions yet.