-1

Well I've gotten myself into a messy complex of objects that refer to each other. It is too complex to show a minimal example so I'll explain the relations between those objects.

I'm also not looking for code review, rather for a method/paradigm I can use to allow deletion without breaking the relational behaviour between those. And preserve the application's state. Yet I also wish to keep the objects separated enough from each other that they work without needing each other.

Environment
The objects I have are the following: Obj_A Obj_B Obj_A_Factory and Obj_B_Factory

Obj_A_Factory is simply a factory for Obj_A. It also keeps track of all instances of this class. And preserves the fact that each Obj_A is "unique". Similar story for Obj_B_Factory - it's a factory for Obj_B that keeps a list (actually those are dictionaries but that's besides the point).

Now Obj_A contains a list with references to (a number of) Obj_B. This list can be empty if no Obj_B is added to Obj_A. Obj_B in response keeps a reference to the "parent" Obj_A it is added too, with the note that null is a complete valid value here: indicating it hasn't been added to any.

This is very similar to a "tree", however with the difference that the nodes do NOT own their childs, rather they refer to it and the ownership is done by an external manager.


Problem
What happens if I wish to "remove an Obj_A or Obj_B". Just removing them from the list in the factory classes would leave references to "inexistant" objects. (Or as GC won't remove them: they would still exist and be refered to, while the rest of the program thinks they are removed).

In a RAII world I would simply use the destructor of Obj_A to "iterate through the list of references, for each Obj_B in the list set parent-reference to null. And the destructor for Obj_B would "go to the parent Obj_A and remove the reference to this object from the list".

Now this works if a destructor is deterministic. In a non deterministic world this can't be used.

Bad solution
The closest thing to destructors for C# is Dispose(). In the dispose method I can do a similar thing as the destructors. Then when I remove an element from the list in the factories I would also call dispose on that element.

This, however, leaves me with another problem: What if Obj_A_Factory or Obj_B_Factory get deleted? (while the others aren't). Then again the list "containing/owning Obj_A" would get deleted; The expected result being that all refered to Obj_A's would be open for garbage collection, and are no longer valid to be refered to.

So I would have to call Dispose() on all Obj_A owned by Obj_A_Factory, when I wish to free Obj_A_Factory. In other words: it also needs to implement Dispose().

This would then escalate, because what about the class that contains the Obj_A_Factory...

Is this the right way to go about it? Or is there some other construct I should use to manage this? Is there a way I can "seal" the cascading of Dispose() so that I can abstract away this memory management (exceptions..) mess?


minimal example
This is a minimal example I can create, before any of the management options. I wish to get the removing working: currently it would break the "state". Notice it doesn't "do" anything with the cardlist etc yet, it just shows the relations between the classes.
Also notice I do not have any code that shows "deletion of the factories" - however I wish to also support this, I do not wish the factories to behave as singletons.

class Obj_A_Deck
{
    public string Name { get; }
    public IList<Obj_B_Card> cardList { get; };
    public Obj_A_Deck(string name) {
        Name = name;
    }
}

class Obj_B_Card
{
    public string Name { get; }
    public Obj_A_Deck MyDeck { get; private set; }
    public Obj_B_Card(string name) {
        Name = name;
        MyDeck = null;
    }

    public void AttachToDeck(Obj_A_Deck deck) {
        if (MyDeck == deck) return;
        MyDeck = deck;
        MyDeck?.cardList.Add(this);
    }
}

class Obj_A_Factory_DeckList
{
    private readonly Dictionary<string, Obj_A_Deck> _deckList = new Dictionary<string, Obj_A_Deck>();

    Obj_A_Deck CreateDeck(string name) {
        var d = new Obj_A_Deck(name);
        _deckList.Add(d.Name, d); //exception guard against duplicates
        return d;
    }

    void RemoveDeck(string name) {
        _deckList.Remove(name);
    }
}

class obj_B_Factory_Inventory
{
    private readonly Dictionary<string, Obj_B_Card> _cardList = new Dictionary<string, Obj_B_Card>();
    Obj_B_Card CreateCard(string name) {
        var c = new Obj_B_Card(name);
        _cardList.Add(c.Name, c); //exception guard against duplicates
        return c;
    }

    void RemoveCard(string name) {
        _cardList.Remove(name);
    }
}
paul23
  • 8,799
  • 12
  • 66
  • 149
  • 3
    `The equivalent of destructors for C# is Dispose()` I don't think that is a correct statement. – hatchet - done with SOverflow Mar 15 '17 at 23:17
  • @hatchet Well the "closest thing like it". - Please don't fall over the fact that I'm not a native English speaker and just update the post for that. (As it isn't important for the rest of the post, just is an introduction as for why I chose to try that method). – paul23 Mar 15 '17 at 23:18
  • 2
    But `destructor` already has a specific, defined meaning in c#. http://stackoverflow.com/questions/13988334/difference-between-destructor-dispose-and-finalize-method – hatchet - done with SOverflow Mar 15 '17 at 23:20
  • As simply as you can: what is the specific reason/need for the need for `ObjA` to maintain a list of references to its `ObjBs`, and what is the reason/need for `ObjB` to maintain a reference to it's `ObjA`? – khargoosh Mar 15 '17 at 23:22
  • I think it's clear that this is a design problem. Modifying the relationship or interaction between `ObjA` and `ObjB` will probably negate your issue. – khargoosh Mar 15 '17 at 23:23
  • It's not clear what the actual problem is. You should include a good [mcve] (and make sure it's _really_ a good MCVE) that clearly shows your scenario, what you're trying to do, and what _specific_ problem you're trying to solve. Your "factories" sound more like caches/memos to me (which is fine). As such, you may prefer to implement them with weak references. The cycle between parent and child won't keep the objects from being collected; but a strong reference in the factories will. So make those weak references, and then when the rest of your program is done with the objects, they'll go away – Peter Duniho Mar 15 '17 at 23:30
  • @khargoosh Well this relation is simply what the program is about: managing which `Obj_B` is part of which `Obj_A`. Say, `Obj_B` are cards and `Obj_A` are decks: The decks don't own the cards, my inventory does. - Now the backreference is simply there to facilitate moving cards, without having to iterate over all `Obj_A` to find the correct one which contains `Obj_B`. – paul23 Mar 15 '17 at 23:31
  • @PeterDuniho: I've deleted my answer, but there's no duplicate proposed yet – Thomas Weller Mar 15 '17 at 23:36
  • @ThomasWeller: I didn't close as a duplicate because, frankly, it's not actually clear _to me_ what the OP is struggling with. My point was simply that if _you_ think that's the answer he needs, _you_ can look up the duplicate and propose that. Each of us needs to evaluate the question as we ourselves understand it (or fail to understand it, as the case may be :) ). I can't vote to close as duplicate because my understanding of it doesn't say I should. But your understanding of it may be different. – Peter Duniho Mar 15 '17 at 23:38
  • @PeterDuniho Updated with the an MCVE "upto" the memory/state problem. I am at that point right now and I need to decide how to tackle this: what is the right tool to manage the state. – paul23 Mar 16 '17 at 00:04
  • If we take this example at *face* value - can you approach this differently? For example - even though a `Card` may be held by a `Player`, it still *belongs* to a `Deck`. This approach would remove the need for a `CardFactory` that retains a list of `Cards` – khargoosh Mar 16 '17 at 00:29

2 Answers2

1

This seems straightforward so maybe I'm missing something.

When the B factory removes a B object it tells the associated A that the B being removed so the A can remove its reference to the B.

When you remove an A object then you remove all its B references and tell the B factory that the references are being removed.

Don't mess with Dispose. It's a trap put in C# for unwary C++ developers.

Ray Fischer
  • 936
  • 7
  • 9
  • This would mean that the factories (registers) not only are dependent on the objects they try to construct/manage: but they are also influential the "other object". Thus no longer would each "module" be a self-standing module. – paul23 Mar 15 '17 at 23:39
  • I whole-heartedly agree with the "Don't mess with Dispose". It remains to be seen whether this addresses the OP's question though. – Peter Duniho Mar 15 '17 at 23:39
  • @paul23: you really need to fix your question so that it makes more sense. As I suggested before, edit it so that it includes a good [mcve] showing exactly what the scenario looks like, and explain _precisely_ what doesn't work about the code (i.e. what does the code do now, and what do you want it to do instead). – Peter Duniho Mar 15 '17 at 23:40
  • You've already described that in some way these objects have a dependency on each other... otherwise why hold a reference? – khargoosh Mar 15 '17 at 23:41
  • @PeterDuniho Well it does "answer", however it means that the objects become even more tightly coupled: now any removal of the object NEEDS to be done by the factory, lest I might break the state of the application. - I rather have objects cleaning up after themselves, so the object itself is responsible of attaching itself to another object: it should hence also unattach itself. – paul23 Mar 15 '17 at 23:41
  • You can also have the individual objects do more of the work. When a B is to be removed then it can tell the A that it's being removed. When an A is being removed then it can tell its Bs that their references are going away. Each object type tells its factory when it needs to go away so each factory only needs to know about its objects. Objects already know about each other so nothing additional is needed. Ta da! – Ray Fischer Mar 15 '17 at 23:42
  • `objB.Remove() { if (this.objA != null { objA.RemoveReference(); } }` etc - I don't know that this changes the coupling between the classes – khargoosh Mar 15 '17 at 23:43
  • @RayFischer but now consider the case that the `Obj_A_Factory` goes out of scope/can be freed: this happening means (logically) that all `Obj_A` inside it can be freed, as an "inventory" - thus a similar problem as `Dispose()`. – paul23 Mar 15 '17 at 23:46
  • @paul23: _"it should hence also unattach itself"_ -- unattach itself from _what_? Assuming the objects are no longer needed by the program, then the only references left keeping them alive are in the factories. If you don't want the references in the factories, then remove those references from the factories. Your description of the object "responsibilities" is unclear, but there doesn't seem to be any fundamental problem here. You need to explain what your code doesn't do now that you want it to do. – Peter Duniho Mar 15 '17 at 23:50
  • @paul23 But why is that a problem? `Obj_A_Factory` calls all `Obj_A.Remove()` methods, which in turn call all `Obj_B.RemoveReference()` methods. This doesn't change the coupling between these classes! – khargoosh Mar 15 '17 at 23:50
  • @paul23, You're thinking in C++. "Go out of scope" is going to be bad so don't assume that is a safe approach. It looks like you want to create an explicit method to release everything or else there will be referenced objects that don't belong in any factory. But there is something that might be useful: a using block and an associated Dispose method. It's not as automatic as a C++ destructor but may solve your problem. You can see a good explanation here: http://stackoverflow.com/questions/212198/what-is-the-c-sharp-using-block-and-why-should-i-use-it – Ray Fischer Mar 16 '17 at 00:01
1

It sounds like you are looking for a WeakReference<T> for your nodes. A WeakReference holds a reference to the managed object while still allowing it to be reclaimed by the garbage collector. If your application is done with all references to the object, but the only reference left is the one in your factory, the garbage collector will collect that instance and the TryGetTarget(out T) method returns false and the reference is null.

The downside of WeakReference<T> is that it is another object that takes up memory. However, it is designed for this type of scenario.

I will caution that if your Obj_A and Obj_B have direct references to each other, then they will still reside in memory as long as there is a direct reference to either object somewhere in your code. In other words, you'll have to test with a memory profiler to find how often you have these objects surviving garbage collection.

Berin Loritsch
  • 11,400
  • 4
  • 30
  • 57