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!
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).