Laden...

Observer - unsubscribe im Dispose()

Erstellt von LaTino vor 7 Jahren Letzter Beitrag vor 7 Jahren 3.030 Views
LaTino Themenstarter:in
3.003 Beiträge seit 2006
vor 7 Jahren
Observer - unsubscribe im Dispose()

Habe mal wieder ein Stück Code aus einem Review, bei dem ich mir nicht sicher bin, was ich davon halten soll - ein paar Meinungen von euch würden mir helfen.

Es handelt sich um eine sehr einfache Observer-Implementierung mit einem etwas ungewöhnlichen Twist:


class Client
{
    private List<Subscriber> _subscribers;
    public void Subscibe(Subscriber subscriber) => _subscribers.Add(subscriber);
    public void Unsubscribe(Subscriber subscriber) => _subscribers.Remove(subscriber);
}

class Subscriber : IDisposable
{
   private Client _client;

   public Subscriber(Client client)
   {
        client.Subscribe(this);
        _client = client;
   }

//idisposable-Implementierung, soweit i.o., bis auf...
   protected virtual void Dispose(bool disposing)
   {
        //...irrelevante Zeilen weggelassen, das Übliche halt
        _client.Unsubscribe(this);
   }
}

Ich habe ein ungutes Gefühl dabei - wobei ich den Finger nicht genau auf den Punkt legen kann, der mir Unbehagen verursacht.

Was denkt ihr?

LaTino

"Furlow, is it always about money?"
"Is there anything else? I mean, how much sex can you have?"
"Don't know. I haven't maxed out yet."
(Furlow & Crichton, Farscape)

T
314 Beiträge seit 2013
vor 7 Jahren

Hmmm ich stell mir folgende Fragen:

  • wieso hat der Subscriber überhaupt eine direkte Verbindung zum Client
  • warum ist er selbst für das Subscriben/Unsubscriben zuständig

Unabhängig davon würde es mir persönlich nicht gefallen, wenn sofort im Konstruktor subscribed wird und ich gezwungen bin zu disposen um zu unsubscriben.

F
10.010 Beiträge seit 2004
vor 7 Jahren

Wenn dein _subscribers die abos in normalen Listen hält ( keine WeakReference ) dann ist das so eigentlich die einzige sichere Möglichkeit keine Speicherlecks zu haben.

Ist übrigens das was ich mal zu dir wegen deterministischem Verhalten zu Dispose sagte.

W
955 Beiträge seit 2010
vor 7 Jahren

* Client sollte Publisher heissen
* warum eigentlich keine Events?

LaTino Themenstarter:in
3.003 Beiträge seit 2006
vor 7 Jahren

Hmmm ich stell mir folgende Fragen:

  • wieso hat der Subscriber überhaupt eine direkte Verbindung zum Client
  • warum ist er selbst für das Subscriben/Unsubscriben zuständig

Ich habe den Teil stark gekürzt, weil er nicht das Problem betrifft, das ich mit dem Code habe. Das ergibt prinzipiell schon Sinn so - bis auf den Punkt, dass man gezwungen ist, zu disposen, um das Abo zu beenden. Vielleicht hätte ich das noch deutlicher machen sollen - Dispose() ist nicht die einzige Möglichkeit, die subscription zu beenden (sowohl client als auch subscriber bieten dafür entsprechende Schnittstellen).

Der Sinn dahinter ist wohl gewesen, das hier zu ermöglichen:


using(var tmpSubscriber = new Subscriber(client))
{
    tmpSubscriber.SpecialProperty = blablubb;
    client.DoSomething();
}

Funktioniert soweit natürlich auch. Ich glaube aber, ich halte das irgendwie für einen Missbrauch von Konstruktor und Dispose(). Die Frage ist - ist das nur ein etwas schräger Stil, oder reisst das potenzielle Probleme auf?

@witte: das war exakt meine erste Frage. Ich kann nur vermuten, dass man sicherstellen wollte, dass die Referenzen auf jeden Fall aufgelöst werden - was FZelle schrieb.
Die Benennung hier ist von mir, wie gesagt. Habe nur versucht, das Prinzip mit möglichst wenig Code klar zu machen.

LaTino

"Furlow, is it always about money?"
"Is there anything else? I mean, how much sex can you have?"
"Don't know. I haven't maxed out yet."
(Furlow & Crichton, Farscape)