How do I structure my code to avoid tight coupling between my parent and child classes?

Question

I'm using a third party product that has a class I can extend to provide a new way for that product to get data (the product is Sitecore but I don't think it's super relevant to the question).

They have a DataProvider class that I can extend:

public class Sitecore.DataProvider {
    public ItemDefinition GetItemDefinition();
    public VersionUriList GetItemVersions();
    // etc
}

I've got a lot of sources of data to access, so I've made a few "data accessor" classes:

public class MyCode.IDataAccessor<TTypeOfData> {
    TTypeOfData GetDataItem(int key); 
    IEnumerable<int> GetDataKeys();
}
public MyCode.WWWDataAccessor : IDataAccessor<WebPages>
public MyCode.CSVDataAccessor : IDataAccessor<Spreadsheets>

I've got a class that maps IDs from the source data to IDs in the third party sysem:

public class MyCode.DataMapper { /* .... */ }

I've got a few classes that extend DataProvider and allow the third party system to access my data:

// these override a lot of methods from Sitecore.DataProvider
public MyCode.WWWDataProvider : DataProvider { /* .... */ }
public MyCode.CSVDataProvider : DataProvider { /* .... */ }

Lots of code is common between WWWDataProvider and CSVDataProvider so obviously I need to break that code out somewhere instead of duplicating it. All of the methods revolve around accessing methods in a given IDataAccessor or DataMapper.

Option one is to have all of those methods on an abstract base class "MyCode.MyBaseDataProvider" that extends Sitecore.DataProvider. Child classes can set the IDataAccessor and DataMapper on that class in their constructor, with something like base(new WWWDataAccessor(), new DataMapper()). This feels pretty messy in that the parent class and child classes are super tightly coupled. I'm also thinking that I should "favour composition over inheritance" just in case I need to change my DataProvider child classes such that they use different logic.

Option two is to leave that base class abstract, and provide a variety of methods that delegate to other objects as appropriate - e.g. I could have an abstract TTypeOfData GetDataItem(int key); method in that base class, and then the child classes just implement it with return this.dataAccessor.GetDataItem(key). But that feels messy in that really all these classes do is cover the logic of what methods to call on those other objects, so I'll just be rewriting out that logic for each of the children - and I'll be duplicating all of the methods on the "helper" objects as thin wrappers in the child classes.

Option three - the option I'm currently going with - is to have a separate "implementation" class. In my child class I do this.implementation = DataProviderImplementation(dataAccessorObj, mapperObj); then I have a lot of overrides like:

public override IDList GetChildIDs(ItemDefinition parentItem, CallContext context)
{
    if (implementation.CanProcessParent(parentItem))
    {
        return implementation.GetChildIDs(parentItem, context);
    }
    return base.GetChildIDs(parentItem, context);
}

There's no chance of using a factory pattern to instantiate these classes; the third party system requires me to give it a fully qualified class name and only string arguments to it in an XML config file, so I need both WWWDataProvider and CSVDataProvider classes to do the work of instantiating the relevant IDataAccessor and DataMapper classes.

Is there a nicer way to do this? Is there a pattern I should be looking into?

Answer 1

I understand your cautiousness. Another option, certainly in c#, is if you have classes that have the same methods or properties, have them implement an interface (if the class does something) or inherit from a base class if its a model, which defines these common methods and properties, and then build extension methods for that interface/abstract. You should be able to remove the these shared methods completely from your objects. I really like this method because it keeps a lot of your "utility" methods in separate extension classes and out of your objects. Please do not confuse an extension class with extending a class, I am not referring to inheritance. Also be cautious not to overuse this method. If a particular piece of functionality isn't common, there really isn't much need to abstract it into an extension method. When extension methods are overused they can often do more harm than good.

And example of an extension class

public abstract class BasePerson
{
    DateTime DOB { get; set; }
}

public class Person : BasePerson
{
}

public class Employee : BasePerson
{
    public StartDate { get; set; }
}

public static class Extensions
{
     public int GetAge(this BasePerson obj)
     {
        return (DateTime.Now - obj.DOB).Years;
    }
}

And to use it

var p = new Person { DOB = DateTime.Now.AddYears(-20); };

var age = p.GetAge();

var e = new Employee { DOB = DateTime.Now.AddYears(-20); }
age = e.GetAge();

The GetAge() extension method is available to both concrete classes (Person, Employee) because they both inherit from BasePerson