views:

82

answers:

1

First of all, I'm not sure if the title accurately describes what I'm referring to, so feel free to leave a comment on what to call this, or just rename if yourself if you have the rep.

Let's say for example I have 2 classes, Book and Library. A Library contains a property that is a list of all the Books it owns. A Book has a property that that is a list of all the Libraries it belongs to.

On Book I have a RemoveFromLibrary method that removes a library from it's list of libraries. I also want that same method to clean up the other end, namely the library's list of Books that it owns. The same goes for the other end, a RemoveBook method on Library that also cleans up that Book's list of libraries that contain it.

First of all, does this make sense? It seems convenient to me for calling code to not have to worry about cleanup and not have to call 2 methods to perform one logical action. I mean it doesn't make sense to ever remove from one list but not the other. At the same time, it could be said that this makes them too tightly coupled, but I'm thinking I'll just refactor to decouple if it becomes a problem.

What I'm not sure about as far as implementing this is if I just call the normal public method, then I'll end up with an infinite loop of each method calling the other. I could make a separate internal property (working in C#) that doesn't cleanup, but it feels like I'm littering the API with a method that's only meant to be called from one other method. Alternatively I could expose the underlying collection as internal, which is a little better, but it still doesn't seem ideal. Is there a better solution or will I just have to go with one of those two, or make sure calling code does the cleanup itself?

+1  A: 

Maybe this will work. Before calling the remove method of the other class, check that it's list of objects (books, for example) still contains the want-to-be-removed object.

BTW this is only pseudocode (may not compile)

class Book
{
    Library[] libraries;

    void RemoveFromLibrary(Library lib)
    {
        libraries.remove(lib);
        if (IsInLibrary(lib) && lib.HasBook(this)) // prevents call-loop
            lib.RemoveBook(this);
    }

    bool IsInLibrary(Library lib)
    {
        return libraries.Contain(lib);
    }
}


class Library
{
    Book[] books;

    void RemoveBook(Book bk)
    {
        books.remove(bk);
        if (HasBook(bk) && bk.IsInLibrary(this)) // prevents call-loop
            bk.RemoveLibrary(this);
    }

    bool HasBook(Book bk)
    {
        return books.Contain(bk);
    }
}

edit: fixed the code based on comments ... I was sleepy while writing it :P

Aziz
Wow... now that you point it out it seems so obvious. Btw, the local remove needs to come before checking/removing from the other list, as written you still have an infinite loop because the if condition is always true. +1 for the concept anyway, and I'll accept if you fix that and no one else posts a better answer.
Davy8
Also in RemoveBook(Book bk) you have book.RemoveLibrary(this) when you mean bk.RemoveLibrary(this).
Davy8
Fixed .. anyway you got the idea :)
Aziz