OOP, Interface Design and Encapsulation [closed]

Closed. This question does 开发者_开发技巧not meet Stack Overflow guidelines. It is not currently accepting answers.

---

We don’t allow questions seeking recommendations for books, tools, software libraries, and more. You can edit the question so it can be answered with facts and citations.

Closed 5 years ago.

Improve this question

C# project, but it could be applied to any OO languages. 3 interfaces interacting:

public interface IPublicData {}

public /* internal */ interface IInternalDataProducer { string GetData(); }

public interface IPublicWorker {
    IPublicData DoWork();
    IInternalDataProducer GetInternalProducer();
}

public class Engine {
    Engine(IPublicWorker worker) {}
    IPublicData Run() {
        DoSomethingWith(worker.GetInternalProducer().GetData());
        return worker.DoWork();
    }
}

Clearly Engine is parametric in the actual worker that does the job. A further source of parametrization is how we produce the 'internal data' via IInternalDataProducer. This implementation requires IInternalDataProducer to be public because it's part of the declaration of the public interface IPublicWorker. However, I'd like it to be internal since it's only used by the engine.

A solution is make the IPublicWorker produce the internal data itself, but that's not very elegant since there's only a couple of ways of producing it (while there are many more worker implementations), therefore it's nice to delegate to a couple of separate concrete classes. Moreover, the IInternalDataProducer is used in more places inside the engine, so it's good for the engine to pass around the actual object.

Also not an option is (obviously) to pass an instance of IInternalDataProducer directly to the Engine constructor. We don't want users of the library (Engine) to know about data producers. It's the worker's responsibility to declare (produce) what data producer should be used.

I'm looking for elegant, type-safe, ideas/patterns. Cheers :-)

EDIT: Based on Jeff's answer, here's a solution:

It's not perfect, because the interface is still 'dirty' with GetData() which the user shouldn't need to see (but my original interface was dirty too), and because it leads to 'interface duplication' (GetData() is declared in 2 interfaces) - you can't have everything.

The next problem is how to clean GetData() out of the IPublicWorker interface then :)

public interface IPublicData {}

internal /* ! */ interface IInternalDataProducer { string GetData(); }

public interface IPublicWorker {
    IPublicData DoWork();
    string GetData();
}

public class APublicWorker : IPublicWorker {

    private IInternalDataProducer dataProducer;

    public APublicWorker() {
        dataProducer = new SomeDataProducer();
    }

    IPublicData DoWork() { ... }
    string GetData() {
        /* Delegation! */
        return dataProducer.GetData();
        /* ********* */
    }
}

public class Engine {
    Engine(IPublicWorker worker) {}
    IPublicData Run() {
        DoSomethingWith(worker.GetData());
        return worker.DoWork();
    }
}

---

You can solve this by encapsulating the data producer within the worker:

public interface IPublicWorker {
    IPublicData DoWork();
    // Callers don't care how a worker gets this data
    string GetData();
}

The call in engine then looks like this:

IPublicData Run() {
    DoSomethingWith(worker.GetData());
    return worker.DoWork();
}

---

Actually, Oleg's solution is not that bad, but can be improved.

Given that you want to put restrictions on the IPublicWorker interface, I'm assuming that you want to control the implementations of IPublicWorker, and provide your users with a specific API to obtain them. If that is the case, you could derive an IInternalPublicWorker from IPublicWorker, and in your Engine's constructor, verify that the IPublicWorker is indeed of the expected type:

public interface IPublicData {}


public internal interface IInternalDataProducer { string GetData(); }

public interface IPublicWorker {
    IPublicData DoWork();
}

public internal interface IInternalPublicWorker : IPublicWorker {
    IInternalDataProducer GetInternalProducer();
}

public class Engine 
{
    IInternalPublicWorker _worker;

    Engine(IPublicWorker worker) 
    { 
        if (!(worker is IInternalPublicWorker)) 
        {
           throw new InvalidOperationException("We don't support workers that were not obtained from our library."); // add some helpful message about which method to use to obtain a worker
        }
        _worker = (IInternalPublicWorker)worker;
    }
    IPublicData Run() 
    {
        DoSomethingWith(_worker.GetInternalProducer().GetData());
        return _worker.DoWork();
    }
}

---

UPDATE
My solution doesn't seem to be correct, at least it doesn't work as i intended. Maybe someone else can get it to work as it would be the (imho) most logical solution.

---

let the internal interface inherit from a public interface. This allows the external parts to only access what you really want accessible while still hiding the internal parts.

public interface IPublicData {}

public interface IPublicDataProducer {}
internal interface IInternalDataProducer : IPublicDataProducer { string GetData(); }

public interface IPublicWorker {
    IPublicData DoWork();
    IPublicDataProducer GetProducer();
}

public class Engine {
    Engine(IPublicWorker worker) {}
    IPublicData Run() {
        DoSomethingWith(worker.GetProducer());
        return worker.DoWork();
    }
}

This allows you to seperate the data for the external and internal users of your interfaces.

---

Edited to reflect requirements from comments:

public interface IPublicData { }

public interface IDataProducer { string GetData(); }

internal interface IInternalDataProducer : IDataProducer { string GetData(); }

internal class InternalDataProducer : IInternalDataProducer
{
    public string GetData()
    {
        throw new NotImplementedException();
    }
}

public interface IPublicWorker
{
    IPublicData DoWork();
    IDataProducer GetInternalProducer();
}

class PublicWorker : IPublicWorker
{
    public IPublicData DoWork()
    {
        throw new NotImplementedException();
    }

    public IDataProducer GetInternalProducer()
    {
        return new InternalDataProducer(); //here you binds PublicWorker to paricular data provider
    }
}


public class Engine
{
    private IPublicWorker worker;

    public Engine(IPublicWorker worker)
    {
        this.worker = worker;
    }

    IPublicData Run()
    {
        DoSomethingWith(this.worker.GetInternalProducer());
        return worker.DoWork();
    }

    private void DoSomethingWith(IDataProducer getData)
    {
        throw new NotImplementedException();
    }
}

---

Why not define GetInternalProducer() as

  object GetInternalProducer();

and use something like

   IInternalDataProducer producer = GetInternalProducer() as IInternalDataProducer.

you would have to check for null pointer, but you don't need to expose IInternalDataProducer anymore.

This way you can provide a factory to generate IInternalProducer objects inside Engine and associate them with IPublicWorker objects or any other code througout the application. Otherwise the client code has to know about IInternalDataProducer thus violating the encapsulation of the Engine.