Ringbuffer implementation

Hallo Freunde,

ich hab für meine zwecke nen RingPuffer (ne endlos doppelverkette liste) geschrieben. Wird verwendet für asyncrone Datenübertragung übers Net als Send Buffer, zum Pollen.

public class CycleBuffer<K, T>
    {

        class CycleElem
        {
            //Members not hidden, because of nestes Elem Class

            //Elem Values
            public T m_Dat;
            public K m_Key;

            //References of next Elemnts
            public CycleElem m_Prev = null;
            public CycleElem m_Next = null;

            //Konstruktor
            public CycleElem(K Key, T Dat)
            {
                m_Dat = Dat;
                m_Key = Key;
            }
        }

        //Current Cycle Elem Position
        CycleElem m_objPos = null;

        //Syncronisation- object for Multithreading
        object objLock = new object();

        //Remember Elem Count
        int m_iCount = 0;

        //Property returns the Object at current Cycle Position
        public T Value{  get { return m_objPos == null ? default(T) : m_objPos.m_Dat; } }

        //Move one elem forward
        public bool Prev() { lock (objLock) { return (m_objPos = (m_objPos != null) ? m_objPos.m_Next : m_objPos) !=null; } }

        //Move one elem backward
        public bool Next() { lock (objLock) { return (m_objPos = (m_objPos != null) ? m_objPos.m_Prev : m_objPos) !=null; } }

        //Check if Key exists
        public bool ContainsKey(K Key)
        {
            return this[Key] != null;
        }

        //Add ne Elem
        public T Add(K Key, T dat)
        {
            if(ContainsKey(Key)) return default(T);

            lock (objLock)
            {

                //Elems = 0
                if (m_objPos == null)
                {
                    m_objPos = new CycleElem(Key, dat);
                    m_objPos.m_Next = m_objPos;
                    m_objPos.m_Prev = m_objPos;
                }
                //Elems > 0
                else
                {
                    CycleElem tmp = new CycleElem(Key, dat);
                    tmp.m_Prev = m_objPos.m_Prev;      // C
                    m_objPos.m_Prev.m_Next = tmp;      // D 
                    m_objPos.m_Prev = tmp;  // A
                    tmp.m_Next = m_objPos;  // B
                   // m_objPos = tmp; //Elem Postion at ne added Elem
                }  
                //Inkrement Elem counter
                ++m_iCount;
            }

            return dat;
        }

        //Get Value from Elem with specified Key
        public T this[K Key]
        {

            get 
            {
                lock (objLock)
                {
                    for (CycleElem e = m_objPos; e != null || e.m_Next != m_objPos; e = e.m_Next)
                    {
                        if (e.m_Key.Equals(Key)) return e.m_Dat;
                    }
                }
                return default(T);
            }
        }

        //Delete Elem from Buffer
        public void Del()
        {
            lock (objLock)
            {
                // One or less elems exists set Pointer(Ref) to null
                if (Count <= 1)
                {
                    m_objPos = null;
                    m_iCount = 0;
                }
                // Two or more elems
                else
                {
                    CycleElem tmp = m_objPos.m_Prev;
                    tmp.m_Next = m_objPos.m_Next;
                    m_objPos.m_Next.m_Prev = tmp;
                    m_objPos = m_objPos.m_Next;
                    --m_iCount;
                }
            }
        }

        // Get elemen Count
        public int Count { get { return m_iCount; } }

        // Check if buffer Empty
        public bool IsEmpty { get { return m_iCount == 0 && m_objPos == null; } }

    }

Würde mich über Feedback und Kritik freuen, da ich noch nich lang mit C# und Exceptions bewandt bin, frage ich mich wo hier am ehesten exception abgefangen werden müssten!? grüße

hajb

Auf den ersten Blick sieht es in Ordnung aus, aber es gibt einige Probleme. Vermutlich hättest Du noch getestet und einige davon gefunden (spätestens im Produktivbetrieb )

Man hätte intern eine existierende Datenstruktur verwenden können, z.B. List<T>, hängt aber sicherlich mit deinen Anforderungen an die Performance zusammen.

Das ist mir aufgefallen:

if(ContainsKey(Key)) return default(T);

Warum default(T)? Ich würde sowas wie eine KeyExistsException werfen, oder überschreiben, je nach Anforderung. Ansonsten denkt der Benutzer deiner Klasse, dass es geklappt hat. => Fehlerquelle.

Auch hier:

public T Value{  get { return m_objPos == null ? default(T) : m_objPos.m_Dat; } }

Man kann nicht unterscheiden ob die Liste leer ist oder default(T) an der aktuellen Position steht.

IsEmpty und Count==0 sollten das gleiche zurückgeben, das tun sie glaube ich nicht immer, weil isEmpty mehr macht.

Ich würde mir mal die Thread-Sicherheit nochmal genauer angucken. Etwa hier:

public T Add(K Key, T dat)
        {
            if(ContainsKey(Key)) return default(T);

            lock (objLock)
            {
            ....

Hier kann es passieren, dass zwei Treads gleichzeitig prüfen ob Key X schon drin ist (was nicht der Fall war), danach würden sie beide (nacheinander) in den gelockten Bereich eintreten und du hättest zwei Elemente mit gleichem Key X.

Dann: Einer fragt Value ab, und der andere macht gerade Next()! Der erste darf sich nicht mehr darauf verlassen, das Value das gleiche ist wie vorher. Auch kann er nicht sichersein, dass wenn er immer Next() sagt und dann Value abfragt, er auch jedes Element durchläuft! Diese Navigation würde ich aus der Klasse rausnehmen. Nicht umsonst dürfen in bei Iterationen z.B. über List<T> die Datenstrukturen nicht verändert werden.

Wenn du die Klasse produktiv einsetzen willst, rate ich viele viele Unit-Tests dafür zu schreiben. Auch "Last"-Tests, also viele Treads die _sehr_ oft/gleichzeitg mal zufällig was einfügen/löschen/abfragen, mal was festgelegtes, etc. Am besten mit 100% Codeüberdeckung.

Exceptions:
Keine Exceptions abfangen! Nur welche werfen. Wenn etwas schief geht, muss der Benutzer der Klasse das merken.

Ok habe das lock problem und die Rückgabewerte wenn Key nich gefunden wurde gändert.

Ich muss navigieren.. der cyclebuffer wird ständig von einem Polling Thread mit next() durchlaufen, und holt sich bei jedem element ein schippe an daten. Wenn an einem elemen alle daten gelesen wurden löscht diese der Polling thread auch .. von ausen wird nur die "Add" funktion verwendent, d.h. während der Polling Thread fleissig am daten schieben ist, wird über die Addpunkt ein neues element in den polling zyklus kreis eingegliedert.

Ja ich habs bischer mit ner list und mit den Enumeratoren.. aber das stört mich.. weil bspw. bei einem Löschvorgang der enumerator "MoveNext()" ungültig ist. Und bei meinerm Cylce buffer gibts das problem auch net:)