Wrapping 3rd party objects that are dependant upon each other and best practices

Question

I'm having a bit of a moment of indecision here and I'd like some perspective on it.

I'm currently wrapping up objects from a 3rd party API (at home for my own project and at work), and I'm doing my best to adhere as closely to SOLID principles as I can. All of this is written using C#. Just to be clear here: this is not a line of business application, I'm making a framework for reuse that wraps up more complicated details of a 3rd party API.

So my issue is this: I have these objects wrapped up in my concrete instances. We'll call the native objects that are wrapped nA and nB, and the wrappers A and B which hold nA and nB respectively:

class A { private nA _nA }
class B { private nB _nB }

nB can't be created without passing a reference to nA. So, to avoid exposing the nA object to the public, I have a factory method to create wrapper B so it can pass in nA to the constructor of nB. So, first question: I'm assuming this is a proper practice? I don't want to end up with a ton of factories or factory methods if I can help it (there's a fair bit to wrap up).

Now, this is all well and good. I can create A and B no problem because of the above. But, now I have another wrinkle. I have to wrap a method called M1(nB) where the parameter is of type nB, which is stored inside of the B wrapper:

public class A { private nA _nA; public M1(B) { _na.M1( ??? );} }

But, nB is inaccessible. So, what's the best practice to pass nB to the nA.M1() method stored in wrapper A?

I could enforce the use of concrete types, and make nB an internal member on wrapper B like so:

public class B { internal nB { get; private set; } }

But this will introduce tight coupling. Does it matter since nA and nB are tightly coupled anyway?

These kinds of problems always give me a headache, so if anyone could spare some advice, that'd be swell.

Also, while I am trying to adhere to SOLID as best as possible, I am aware they are guidelines. They are not hard set rules or a religion after all. So if I have no choice but to break SOLID principles to get what I need, so be it.

Some code to further illustrate my issue:

interface IGraphics
{
   CopyResource(Texture src, Texture dest);
}

interface ITexture
{
   // Stuff for textures that have no bearing on this example.
}

class Graphics
   : IGraphics
{
   // Native object (or object that wraps a native object).
   private ID3D11Context1 _d3dContext;

   public CopyResource(Texture src, Texture dest)
   {
      // Do validation, etc...    

      // How can I do this with interfaces?  
      // Internals are not allowed on interfaces by the language
      _d3dContext.CopyResource(src._d3dTexture, dest._d3dTexture);
   }
}

class Texture
   : ITexture
{
   // Native object (or object that wraps a native object).
   private ID3D11Texture2D _d3dTexture;
   // Other stuff
}

Answer 1

But this will introduce tight coupling. Does it matter since nA and nB are tightly coupled anyway?

No, it doesn't matter as long as they are internal classes, accessed publicly via interfaces. It's like what goes on inside your classes: if Foo calls a private method, Bar, it's tightly coupled to Bar. But if you later break that coupling and change it to call FooBar and Baz, nothing else is affected as the coupling is encapsulated within your class.

The same applied to what goes in inside A and B. As long as the public methods reference IA and IB only, the internals of A and B are free to intertwine themselves with nA and nB. In fact, not only are they free to do this, they are expected to: it's their job as wrappers.

Just make sure that those wrappers are exposed to the rest of the system via interfaces, IA and IB. That way, the wrappers (and thus the 3rd party libraries) can be mocked when needed. If you don't do that, your wrappers aren't doing anything useful as the rest of the code is still coupled to nA and nB, via an obfuscation layer, rather than an abstraction layer.

Update

Looking at your updated code example, I'm going to back track a bit, as interfaces may be more trouble here than they are worth. When wrapping classes like this, I personally follow the following "rules":

1. Try to expose all of the wrapped classes via just one interface, encapsulating all the data types etc behind the wrapper. This is the ideal situation, but rarely is practical. It won't work here.

2. Ensure that, eg in your case ITexture, defines enough properties to allow you to create a Texture (and thus a ID3D11Texture2D) from any implementation of ITexture, so that you can do things like:

   public CopyResource(ITexture src, ITexture dest)
   {
       // Do validation, etc...    
   
      _d3dContext.CopyResource(CreateRealTexture(src).D3dTexture,
                               CreateRealTexture(dest).D3dTexture);
   }
   

   with D3dTexture then being an internal property.

   That then allows you to still hide all the real data types, only exposing them via interfaces. The downsides to this are that it can make the interfaces complex and there's a big overhead in creating the "real" types on the fly. This again is unlikely to be of use to you here.

3. Do away with interfaces and use factories to create instances, as you suggest. At this point though, I'd say stop and ask yourself "why?" What is the point of such wrappers? They are a lot of work to create and maintain. They don't help much with testing, DI etc (due to the lack of interfaces).

   If the reason for creating them is so that "I can swap out the graphics library for a different one, as I've added an abstraction to their access". Then I'd say, don't waste your time. The chances of a different graphics library being able to slot in behind your wrapper classes with no changes to those wrappers is vanishingly small. Just access the libraries directly and rewrite if you change.

So there's no perfect solution here. Ask yourself if the wrappers are genuinely of value. If they are, then my gut reaction in this particular case would be to change the likes of:

private ID3D11Context1 _d3dContext;

to be internal items. That way, behind the scenes, your wrapper classes can access all of the underlying types with ease.

Answer 2

Three thoughts for you, OP.

Use an anemic class

If B is just a DTO, there is no reason to be able to mock it, and an interface is not needed (a unit test can just use new and put in mock data). For example, if B is a custom structure designed to hold a date/time stamp, and it contains only fields and properties without behavior, go ahead and expose it as class B instead of as interface IB. It's OK for the client to use new instead of getting every single instance from your library, and unit tests can do the same, as long as the class is a dumb data container.

Use an ID to reconstruct the original object

Don't worry about keeping like-for-like with every single pattern in use by the objects you are wrapping. The point of a wrapper is to expose only what is needed by your client, and in a manner that is easy to use and difficult or impossible to mis-use. It's OK if things change.

For example, it might make more sense to expose a method that accepts an object identifier than an object itself. So instead of

void Method1(ICustomer customer);

you could expose

void Method1(int customerID);

Now internally, method1 will have to use that customerID to construct an (internal) Customer object, compatible with the library you are attempting to wrap. I would suggest it can just construct it on the fly.

void Method1(int customerID)
{
    var c = internalLibrary.GetCustomer(customerID);
    c.InternalLibraryCall();
}

Use internal interfaces

You have a powerful tool at your disposal: internal interfaces. Not internal interface members (as you have discovered, these are not allowed) but the interface itself.

public interface ICustomer
{
    int CustomerID { get; }
}

internal interface ICustomerEx
{
    InternalCustomer GetOriginalCustomerObject();
}

public class Customer: ICustomer, ICustomerEx  //Client won't see ICustomerEx
{
    InternalCustomer _customer;

    internal Customer(InternalCustomer c)
    {
        _customer = c;
    }

    InternalCustomer ICustomerEx.GetOriginalCustomerObject()
    {
        return _customer;
    }
}

Then the method could look like this:

public void HandleCustomer(ICustomer customer)
{
    var c = customer as ICustomerEx;
    var internalC = c.GetOriginalCustomerObject();
    c.InternalLibraryCall();
}