creating a concreate implemetation inside a method

Question

I have this code:

public bool IsWorkAddedWIthConcreteImplementation()
{
 SomeAgentClass agentClass = new SomeAgentClass();
 bool param1 = true;
 string param2 = "Hello";
 DataTable dt1 = agentClass.ProvideData(param1, param2);
 SimpleDAl da = new SimpleDAl();
 DataTable dt = da.ProvideData(dt1);
 bool isDataSentSuccessfully = agentClass.SendDataToClient(dt);
 IUnImplementedInterface dalInterface = new SimpleDAl();
 if (dalInterface.ReturnMeTrue(dt1))
 {
 return true;
 }
 else
 {
 return false;
 }
}

And the implementation

 IUnImplementedInterface dalInterface = new SimpleDAl();

Should I have passed the interface as a parameter to method or in the class constructor? What will happen to the method signature or class constructor signature if more interfaces are introduced ?

Update:

In the actual implementation new SimpleDal() class is replaced by DaFactory.GetUnImplementedInterface(), which is returning me the concrete instance of IUnImplementedInterface.

Now the DaFactory is using Activator.CreateInstance() in order to give me correct instantiation.

Update2

One thing I forgot to tell was, that I am passing the DaFactory instance in the constructor of this class, and later using this factory instance to create concrete IUnImplementedInterface instance.

Update3 and Solution

Finally my code structure will now be having a IUnImplementedInterface instance in the class rather than the Factory instance.So now the caller of this class will have to pass the correct concrete implementation of this interface in its constructor.

• p.s.w.g: What is this code trying to accomplish? The only suggestion I could make without more context is to use return dalInterface.ReturnMeTrue(dt1) instead of if..else.
• Devesh: This method is not a real implementation. It is just created to understand the common mistakes I make, I found out that during unit testing I wasn't able to test this method, so I asked You people whether the code is correct or has some problems in it.
• p.s.w.g: It's impossible to determine if this code is 'correct' without knowing the intent. There's nothing inherently wrong with it it, although the way you use the classes seems strange. If you give us some broader context we may be able to give you an answer.
• Devesh: updated my question as asked. Hope this sheds more light on the context I am talking about.
• Devesh: Thanks a lot for everyone, I don't know which one to choose for answer but, all these answers have been helpful. I have now updated my actual code to adhere to Law of Demeter

Answer 1

I think Factories are a pretty common way of dealing with these situations whether it's right or not. Dependency Injection is a fantastic way to manage changing interfaces but can easily slip down a path where the class starts to violate Single Responsibility Principle or the number of variables injected become ridiculous. Passing via constructor or method really depends on the needed life span of the object in the class but also remember Uncle Bob Martin says the best number of parameters is 0, the next best is 1, etc..

Take a look a Test Driven Development. This guide "Writing Testable Code" by Jonathan Wolter, Russ Ruffer, Misko Hevery has served me well and should help you pull those instantiations from your method. When you write methods to be testable many of these questions solve themselves.

• Devesh: I consider this as an answer because it gives me a Link to Writing testable code again further in this link it discusses regarding Law of Demeter.

Answer 2

This article is a great overview of Inversion of Control (IoC), and why it makes code easier to work with. But here's an example in the context of the code you posted. Imagine you are working on the SimpleDal class and decide that you have some configuration setting that you need to pass in. So now your constructor looks like this.

public class SimpleDal:IUnImplementedInterface
{
 public SimpleDal(string configSetting)
 {
 ...do something with it
 }
 ...
 public bool ReturnMeTrue(DataTable dt1){...}
}

This is not an issue in itself, except for now your line

IUnImplementedInterface dalInterface = new SimpleDAl();

needs to have that string in the constructor call (or you could add another constructor that called it with a default value). The point here is that in the end all your calling code really cares about is the IUnImplementedInterface.ReturnMeTrue() method. You "should" be able to replace SimpleDal with any other class that implements that interface (polymorphism). So, by deferring the creation of the IUnImplementedInterface instance to a level above the calling code or by using a factory method, you separate the concern of creating a dependency and using that dependency. The two most popular methods of achieving that are Dependency Injection, or Factory methods.

• Devesh: @matt I didn't mentioned that I was using factory methods to create concrete instances. Please see my updated question
• Devesh: Thanks a lot Matt, my next step would certainly be using Ioc and making them configuring my classes, if that is possible :)