Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Is there a better design option?

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?

like image 413
Matt Avatar asked Apr 04 '12 14:04

Matt


People also ask

What are design options?

A design option set is a collection of possible solutions for a particular design problem. Each design option set contains one primary option and one or more secondary options. Create an option set with two different options to explore alternate designs.

How do I change my design options?

Click View tab Graphics panel (Visibility/Graphics). In the Visibility/Graphics dialog, click the Design Options tab. For the appropriate design option set, in the Design Option column, select Automatic. Click OK.

What do design options help accomplish?

Design options allow us to create multiple design alternatives to address a design problem. This can be accomplished all within a single model and we can easily move back and forth among those variations.

Where are Revit design options?

Setup a Design OptionOn the Manage tab within Revit, select the Design Option button.


3 Answers

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.

like image 97
John Alexiou Avatar answered Oct 02 '22 14:10

John Alexiou


"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.

like image 28
David Nelson Avatar answered Oct 02 '22 15:10

David Nelson


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

like image 31
Ivo Avatar answered Oct 02 '22 16:10

Ivo