1

I got a class "WorkOrderTaskCreator" that creates objects of the type Task and adds them to third class called WorkOrder. It goes like this

class WorkOrderTaskCreator
{
    ParentWorkOrder ParentWorkOrder { get; private set; }
    public WorkOrderTaskCreator(ParentWorkOrder parentWorkOrder)
    {
        this.ParentWorkOrder = parentWorkOrder;
    }

    public void CreateTaskAndAssignItToWorkOrder(TaskCreator taskCreator)
    {
        var task = taskCreator.CreateTaskInOrder(ParentWorkOrder);
        AddTaskToWorkOrder(task);
    }

    private void AddTaskToWorkOrder(W6Object task)
    {
        //DoSomeTuff
    }
}

TaskCreator is an interface that exposes just the method CreateTaskInOrder, I was tempted to substitute the interface by a delegate. But I do have several classes that implement very different logic inside CreateTaskInOrder.

Is passing an interface on the method a good idea or should I rethink my design?

8
  • I'm having trouble seeing this as DI. There's no dependency on the object here beyond local scope. Are you passing in different TaskCreators at different times?
    – JimmyJames
    May 31 '17 at 14:23
  • Yes, I am. If this wan't the case I wouldn't be tempted to change it for a delegate, nor I'd be using an interface: I'd just define the method inside the class.
    – Zalomon
    May 31 '17 at 14:24
  • 1
    A delegate would be a better decoupling, unless your interface has only one member. In which case it is simply a delegate in disguise. I don't see any advantage of one member interfaces over delegates. May 31 '17 at 17:58
  • It is a one member interface, and I understand your point but dunno: it feels more right to me passing the interface; the logic needed for that one method defined in the interface is not the kind of thing that I'd pass as a Lamba. I guess that, to me, if you wouldn't use a lambda to pass to the method then you shouldn't use a delegate.
    – Zalomon
    Jun 1 '17 at 7:46
  • @Zalomon, absolutely disagree with your last statement. If your method is only dependent on CreateTaskInOrder, then inject that via a delegate, rather than via an interface. C# has special syntax to allow short-hand specifying of methods as delegates, and delegates predate lambdas by a long way. So your claim "if you wouldn't use a lambda to pass to the method then you shouldn't use a delegate" is just plain wrong IMO.
    – David Arno
    Jun 1 '17 at 12:10
4

It is a perfectly valid approach to pass an interface to a method. In fact, I see this as a hallmark of good design.

The only thing about this that seems potentially worth revising is the fact that you pass the task creator to the child which passes the parent to the task creator. You might want to just pass the task creator to the parent and have it create the task. You might then be able to remove the need to pass the parent to the task creator. Hard to say without more info on the larger design, though.

2
  • Yeah, I understand the concern and I share it: it smells pretty bad; issue is that I'm wrapping some legacy code that uses a third party API and I still haven't figured out how to improve that.
    – Zalomon
    May 31 '17 at 14:43
  • Wrapping may be the answer. IMO you can survive messiness in small scopes. Compartmentalizing it and hiding it is the answer. It's like pipes and wiring. You don't worry too much about the aesthetics because it will be hidden behind drywall and fixtures.
    – JimmyJames
    May 31 '17 at 15:00

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.