C# Dependency between classes and IDisposable

Question

Suppose I have 2 classes, A and B. Class A is responsible for "finding/generating" objects of B, so it has some methods that create and return instances of B. Class B has methods that find objects of B that are somewhat related to its instance. Say finding its parent or siblings.

In order to illustrate the situation, suppose that class A is PersonFinder (the only class throuth which one can find a Person) and class B is Person (has children, instances of Person, who can only be found throuth PersonFinder).

The way I thought I could implement this is:

Class A/PersonFinder:

public class PersonFinder
{
    //For the sake of simplicity suppose each person has an unique name
    public Person FindPersonByName(string name)
    {
        //Some code
        return new Person(name, this);
    }
}

Class B/Person:

public class Person
{
    public string Name {get;set;}
    private string childName {get;set;} //For the sake of simplicity, suppose each person has a child and that it knows the its name
    private PersonFinder _personFinder {get;set;}

    public Person(string name, PersonFinder personFinder)
    {
        Name = name;
        _personFinder = personFinder;
    }

    public Person GetChild()
    {
        return _personFinder.FindPersonByName(childName);
    }
}

It works, but I have two questions about it:

1- Is this a proper design? Is there a better way to do it?;

2- Suppose class A/PersonFinder implements IDisposable. According to MS documentation, if a class has an instance of a class that implements IDisposable, then it should implement IDisposable as well. So should class B/Person implement it? If so, then calling Dispose() on an instance of Person would dispose the PersonFinder of all Persons... Would this be an exception to this rule?.

Thanks in advance!

Answer 1

2 - Suppose class A/PersonFinder implements IDisposable. ... Would this be an exception to this rule?.

Implementing IDisposable and disposing is about ownership, i.e. who owns and controls the lifetime of the dependency. In this particular case Person instance will not have the ownership of passed PersonFinder which potentially will be shared between different instances, so Person should not dispose it (and subsequentially implement IDisposable) in this case.

If you are instantiating an instance of PersonFinder per Person like new Person(..., new PersonFinder()) then you should implement the IDisposable and dispose. If both patterns can be present then you can add additional parameter bool dispose which will control the usage (this pattern can be seen in some libraries including framework ones like StreamReader's leaveOpen).

Somewhat similar is the build-in DI and the factory pattern relationship, for example EF Core's DbContext factory - when something (like DbContext) is created via DI then the container has ownership of the lifetime and it will handle disposing IDisposable's, but for the DbContext factory pattern it is not the case:

// Notice that the DbContext instances created in this way are not managed by the application's service provider and therefore must be disposed by the application.
using (var context = _contextFactory.CreateDbContext())
{
    // ...
}

1- Is this a proper design? Is there a better way to do it?;

This is bit more opinionated question. Personally I would go with slightly more decoupled design by making childName public and when child is needed grabbing the instance of PersonFinder and Person and just use the corresponding methods, (i.e. somePersonFinder.FindPerson(person.ChildName)) but without full understanding of the code is actually hard to tell (though if you can't/don't want to expose childName in some way for some reason then the only alternative option is to pass PersonFinder to the GetChild method).

Answer 2

You only need to implement IDisposable if your PersonFinder has a dependency on another disposable object or unmanaged resource for its entire lifetime, i.e. it gets injected or created during construction. If you're simply going to use this other object within one method and it can be safely created and the resources released after each invocation, a using statement will do all the work without you needing to implement IDisposable in your class.

In terms of whether you have the right approach to classes A & B, I'd probably switch around the responsibility of finding the child, as I'm not a fan of the Person being dependent on the PersonFinder. Instead I would publicly expose the child name and have a new method on the PersonFinder to get the child based on a provided Person:

public class PersonFinder
{
    public Person FindPersonByName(string name)
    {
        // Some code
        return new Person(name);
    }

    public Person FindChild(Person parent)
    {
        var childName = parent.ChildName;

        // Some code
        return new Person(childName);
    }

    // Or if there is no difference between a parent and child
    public Person FindChild(Person parent)
    {
        return FindPersonByName(parent.ChildName);
    }
}