Is there a better design option?

Question

^{Disclaimer: I would love to be using dependency injection on this project and have a loosely coupled interface-based design across the board, but use of dependency-injection has been shot down in this project. Also SOLID design principles (and design patterns in general) are something foreign where I work and I'm new to many of them myself. So take that into consideration when suggesting a better design to this problem.}

Here is a simplified version of the code I'm working on, and as such it might seem contrived. If so I apologize. Consider the following classes:

// Foo is a class that wraps underlying functionality from another 
// assembly to create a simplified API. Think of this as a service layer class,
// a facade-like wrapper. It contains a helper class that is specific to
// foo. Other AbstractFoo implementations have their own helpers.

public class Foo : AbstractFoo
{
    private readonly DefaultHelper helper;
    public override DefaultHelper Helper { get { return helper; } }

    public Foo()
    {
        helper = new Helper("custom stuff");
    }

    public override void Operation1(string value)
    {
        Console.WriteLine("Operation1 using " + value);
    }

    public override void Operation2()
    {
        Console.WriteLine("Operation2");
    }
}

---

// Helper derives from a default implementation and allows us to
// override it's methods to do things specific for the class that 
// holds this helper. Sometimes we use a custom helper, sometimes
// we use the default one.

public class Helper : DefaultHelper 
{
    private readonly string customStuff;

    public Helper(string value)
    {
        customStuff = value;
    }

    public override void DoSomethingHelpful()
    {
        Console.WriteLine("I was helpful using " + customStuff);
    }
}

Say these two class are used as follows:

    // foo referenced and used in one part of code
    var foo = new Foo();
    foo.Operation2(); // or foo.Operation1();

    // some other point in the program where we don't have a reference to foo
    // but do have a reference to the helper
    helper.DoSomethingHelpful();

---

However I now find out that I also need to perform foo.Operation1 in some implementations of helper.DoSomethingHelpful();? Potential workarounds I thought of would be:

1. Have foo and helper have a bidirectional relationship. So that in DoSomethingHelpful we can call foo.Operation2
2. Have foo implement IHelp interface and move the "helper" code into foo
3. Use delegation and pass the method Operation2 as an Action<string> delegate into the constructor of Helper.

None of these approaches seem to be ideal (though I've pretty much determined I don't like option 1 and am worried about maintainability with option 3 if we find out later we need to pass in more delegates). This makes me wonder if there is a problem with the initial design of the Helper/Foo combo. Thoughts?

Answer 1

"None of these approaches seem to be ideal (though I've pretty much determined I don't like option 1 and am worried about maintainability with option 3 if we find out later we need to pass in more delegates). This makes me wonder if there is a problem with the initial design of the Helper/Foo combo."

You're exactly right - there IS a problem with the design of Helper and Foo. The basic Foo/Helper relationship as you initially described it is fine, and is a common pattern when you have to wrap other objects that you do not control. But then you say:

"What if I find out that I also need to perform foo.Operation1 in some implementations of helper.DoSomethingHelpful();?"

This is where we have a problem. You started out describing a relationship where Foo is dependent on Helper; now you are describing a relationship where Helper is dependent on Foo. That immediately tells me that your dependency relationships are tangled up. Dependency relationships between objects should only go one way; in fact dependency injection relies on this.

Answer 2

How about a casual ("uses") relationship:

public class Helper : DefaultHelper 
{
    private readonly string customStuff;

    public Helper(string value)
    {
        customStuff = value;
    }

    public override void DoSomethingHelpful(AbstractFoo foo)
    {
        foo.Operation1();
        Console.WriteLine("I was helpful using " + customStuff);         
    }
}

So you modify the abstract helper to expect a reference to the proper Foo implementation.

Answer 3

I think you have what you need. Try not to design for the "just in case I need it later" and don't fix what is not broken. If in the future you need to use Operation1 from your helper, then add it as a dependency on the constructor (as you suggested), or just pass it to the method you are calling. It will depend on the scenario, and you will have it when you actually need something.

EDIT: changed the "Try not to design for the future" as it doesn't seem what I want to say.

EDIT again due changes in the question

You could so something like this:

helper.DoSomethingUsefulWith( foo );

so your helper method will receive the dependency it needs in order to work

Answer 4

I think all your solutions are good; they just offer different capabilities. These differences don't matter too much now but are likely to in the future.

You prefer the second one, and your instincts are the best guide here, you knowing more than the rest of us about your code's future. I like your second solution the best just because it gets rid of a class and is simpler. Due to it's simplicity, if you have to do something else later, you won't have to throw away a lot of work.

The first method lets you play games with different Helper (IHelper?) instances and subclasses. The last method adds a lot of flexibility to Helper. (Although it may add so much you don't need Helper, just the method you're passing to it.) You can switch to using them later if either seems to solve more of the future's unguessed problems.