RAII Helperklassen in C#

inflames2k

Gut, das mit dem Returnen des MemoryStreams hab ich jetzt übersehen. Alloc macht dennoch keinen Sinn.

Shade Of Mine

Ich denke Alloc ist wohl so kompliziert, damit er sauber disposed wenn das new DisposeGuard() eine exception wirft. Was eigentlich nur in OOM situation passieren kann...

hustbaer schrieb:

return Interlocked.Exchange(ref m_target, null);

Brauchst du hier wirklich Thread-Safety? Es ist schliesslich allem voran ein Scope-Guard.

hustbaer

theugly schrieb:

hustbaer schrieb:
return Interlocked.Exchange(ref m_target, null);
Brauchst du hier wirklich Thread-Safety? Es ist schliesslich allem voran ein Scope-Guard.

Das Interlocked.Exchange hab' ich aus einer Klase übernommen wo es wirklich nötig ist, weil potentiell mehrere Threads das selbe Objekt mehrfach disposen können.

Im DisposeGuard ist es wirklich unnötig.

----

Über die komplizierte Alloc Funktion kann man geteilter Meinung sein (und ja, der Grund ist natürlich das new in der Funktion). Ich persönlich mag sowas einfach nicht unsauber schreiben wenn es auch relativ einfach sauber geht. Und da es so leicht ist wenn man nur 1x ScopeGuard.Alloc passend implementiert hat...

Dravere

Wieso ist eigentlich das Target-Property nicht read only? Also so:

public class DisposeGuard<T> : IDisposable
        where T : class, IDisposable
{
    public T Target { get { return m_target; } }

    public DisposeGuard(T target)
    {
        m_target = target;
    }

    // ...
}

hustbaer

Wieso sollte/müsste sie es sein?
Ich müsste den Code durchsuchen ob ich den Setter irgendwo brauche, aber ich sehe einfach keinen Grund ihn wegzulassen.

GPC

hustbaer schrieb:

Wieso sollte/müsste sie es sein?

Aus meiner Sicht wäre es eher unsinnig, der Target-Property nachträglich etwas anderes zuzuweisen. Schließlich wird ein DisposeGuard ja nur über die Alloc-Methode erzeugt und kapselt hierbei ein bestimmtes Target.

inflames2k

Sehe ich ähnlich wie GPC. Denn wer stellt sicher, dass der vorherige Wert von Target wirklich korrekt Disposed wird?

Man stelle sich folgendes überspitztes Beispiel vor:

using(var guard = DisposeGuard.Alloc(new MemoryStream())
{
    .. // irgendwelche arbeiten
    using(FileStream fs = File.Open(path, FileMode.Open)
    {
       guard.Target = fs;
       // irgendwelche arbeiten
    }   
}

Es ist so nicht mehr sichergestellt, dass der MemoryStream korrekt Disposed wird. - Soetwas würde ich sofort unterbinden.

hustbaer

Ja, man kann damit Unfug bauen.

Man kann damit aber sowieso Unfug bauen, z.B. kann man immer ReleaseTarget zu früh aufrufen.

Mein Ziel war nicht ein Hilfsmittel zu entwerfen das man möglichst nicht falsch verwenden kann, da ich der Meinung bin dass das hier gar nicht möglich ist.
Mein Ziel war ein Hilfsmittel zu entwerfen das man möglichst einfach und möglichst flexibel korrekt verwenden kann. Und dazu gehört mMn. auch dass man das Target nachträglich zuweisen kann. Weil mMn. nicht viel Hirnschmalz dazugehört zu verstehen was für einen Effekt das hätte.

Beispiel

Foo Fun()
{
    using (var fooGuard = new DisposeGuard<Foo>())
    {
        foreach (var stuff in m_stuff)
        {
            if (...)
            {
                fooGuard.Target = new Foo(stuff);
                break;
            }
        }

        if (fooGuard.Target == null)
            throw ...;

        DoSomeStuffWith(fooGuard.Target);

        return fooGuard.ReleaseTarget();
    }
}

Ich weiss, das kann man schöner lösen (z.B. über die LINQ Extension Methoden .Where und .First -- bzw. auch über eine eigene Hilfsfunktion), wobei dann auch die Notwendigkeit entfällt .Target nachträglich zuzuweisen. Ich wollte diese Art der Verwendung nur trotzdem nicht ganz ausschliessen.

hustbaer

Weiterer Anwendungsfall wo ich DisposeGuard häufig einsetze: Konstruktoren von IDisposable Klassen.

class Foo : IDisposable
{
    public Foo()
    {
        using (var disposeGuard = DisposeGuard.Alloc(this))
        {
            m_disposableResource1 = ...;
            m_disposableResource2 = ...;
            m_disposableResource3 = ...;
            disposeGuard.ReleaseTarget();
        }
    }
}

Dravere

@hustbear,
Mir ging es vor allem darum zu verstehen, ob das irgendeinen Zweck verfolgt. Wenn es keinen Zweck hat, dann würde ich es weglassen. Eben weil es nicht benötigt wird, wie du es selbst schreibst. Wozu diese Zuweisungsmöglichkeit drin lassen, wenn man damit nur Unsinn machen kann?

hustbaer

Weil mir, wie ich schon geschrieben habe, Fälle einfallen wo man es doch braucht. Ob man die umschreiben kann so dass man es doch wieder nicht braucht, ist dann wieder ne andere Frage

Aber egal. Meine Variante hat es drin. Wer die Klasse in eigene Projekte übernimmt kann den Setter ja weglassen - genau so wie das Interlocked.Exchange EDIT: und das volatile /EDIT.

Dravere

hustbaer schrieb:

Weil mir, wie ich schon geschrieben habe, Fälle einfallen wo man es doch braucht.

Kannst du Beispiele nennen? Rein aus Interesse, weil mir keine einfallen.

hustbaer

Naja z.B. wie schon oben skizziert.

Oder so Sachen wie

Thingy GetCoolestThingy()
{
    using (var coolestThingyGuard = new DisposeGuard<Thingy>())
    {
        foreach (...)
        {
            using (var someThingyGuard = DisposeGuard.Alloc(CreateSomeThingy(...)))
            {
                if (coolestThingyGuard.Target == null || IsCooler(someThingyGuard.Target, coolestThingyGuard.Target))
                {
                    coolestThingyGuard.Dispose();
                    coolestThingyGuard.Target = someThingyGuard.ReleaseTarget();
                }
            }
        }

        // ...

        return coolestThingyGuard.ReleaseTarget();
    }
}

Auch das könnte man natürlich ohne "re-seatable" Target lösen -- z.B. indem man der DisposeGuard<T> Klasse eine Swap Funktion verpasst.
Oder eine explizite ReplaceTarget Funktion.

Dravere

Ah sorry, ich bin anscheinend blind. Hab das vorherige Beispiel irgendwie übersehen. Aber ja, ich würde da schon eher eine Funktion hinpacken. Muss nicht mal ein Swap sein, könnte auch ein Realloc sein oder sowas.

public class DisposeGuard<T>
{
  public void Realloc(T newTarget)
  {
    Dispose();
    m_target = target;
  }
}

Damit wäre gewährleistet, dass das alte Target sauber aufgeräumt wird. Wer das nicht möchte, kann immer noch vorher ein ReleaseTarget aufrufen.

Vielleicht wäre es hier aber dann auch sinnvoll eine Aufteilung zwischen Dispose und DisposeTarget zu machen. Heisst Dispose ruft DisposeTarget auf und Realloc ruft DisposeTarget auf. Nicht nötig, aber klarer. Aber gut, das sind Details.

hustbaer

Dravere schrieb:

Ah sorry, ich bin anscheinend blind.

Haha, macht nix, und sicher nicht so blind wie ich oft