Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

We have a business logic layer (BLL) that is tightly coupled to our data access layer (DAL). We make calls like this:

using (FooData data = new FooData())
{
  data.DoSomething();
}

It's important to note that all of our data classes are internal, and they're in the same assembly as the logic classes, so that only the BLL can access the DAL.

In order to decouple these (to help facilitate unit testing), one idea is to create IDataClass interfaces, like IFooData. At first I thought it may be a problem because we'd have to make our data classes public to implement the interfaces. But we should be able to keep the data classes internal, even though we still need the methods to be public:

public interface IFooData
{
  void DoSomething();
}

internal class FooData : IFooData  // Notice the class is internal
{
  public void DoSomething();  // Method is public, but that's ok because class is internal
}

So even though the method is public, since the class itself is internal, we should still allow only our BLL access.

Is there anything inherently wrong with this approach? Is there a better way to abstract the DAL, for unit testing, without opening the DAL to the world by making it public?

share|improve this question
1  
What are you trying to achieve by keeping your DAL locked away like this? Why keep it secret? It exposes a bunch of methods, it shouldn't care exactly who is calling them. If you have an issue with it being called from the wrong place then you have a code review problem. –  slugster Aug 19 '12 at 1:01
    
By making the methods internal, only the BLL can access it, which forces all developers to go through the BLL. When you have a large team of developers, anything you can do to help minimize mistakes is a good thing. –  Bob Horn Aug 19 '12 at 2:47
    
I realise that you are trying to do a certain amount of hand-holding on the developers that use the code, but in doing so you have created yourself another problem. Which is why I made my comment - there is nothing wrong with having the methods public (it is a normal practice). If people are using them in the wrong way then you have a code review problem. –  slugster Aug 19 '12 at 5:08
    
I disagree. Why have internal and private modifiers at all then? Why not just make everything public? –  Bob Horn Aug 19 '12 at 12:43
    
Bob you have totally missed the point. The DAL exposes public stuff via a CONTRACT which is your interface. Standard OO practice means it is oblivious to who is calling it. Let's say it one more time: you can make the class internal, but you don't need to. The rest of the world manages to get by with this approach, why not you? You've also shot yourself in the foot the moment you have a different BL or service that needs to call the same DAL - are you just going to keep adding friend assemblies? –  slugster Aug 19 '12 at 13:24

5 Answers 5

up vote 12 down vote accepted

Extracting an interface and ensuring the BLL is using only interface types is a good way forward to make the BLL testable without the database.

What you are doing is absolutely fine.

share|improve this answer
3  
I'll add that what you're doing is fine, but it's just the first step. If what you're doing now is using (IFooData data = new FooData()) in your BLL, you haven't really made any change except for introducing the interface. You'll now have to find a way to instantiate the IFooData externally, otherwise your BLL tests will still be coupled to the DAL. –  Avner Shahar-Kashtan Aug 20 '12 at 11:29
    
Agreed. I'm actually doing something like this now: DataAccessFactory.GetDataInterface<IFooData>().DoSomething(); –  Bob Horn Aug 20 '12 at 15:49

You can use the InternalsVisibleToAttribute in order to break your DAL out from your Business Layer, but still keep the DAL methods internal. You'll end up creating your DAL as a friend assembly.

This may not be a great approach as you would have to specify in your DAL's Assembly.cs file which assemblies can access the DAL. So there's still some coupling involved (DAL knowing about BLL).

So, to answer your question, there isn't anything wrong with your approach. But taking advantage of friend assemblies can provide you a bit more abstraction and probably help you out with making your code more testable.

I hope this helps.

share|improve this answer
    
I don't really need to remove the DAL from the BLL; I just need to implement an interface. –  Bob Horn Aug 19 '12 at 0:33
    
By the way, thanks. I'm familiar with InternalsVisibleTo, and if I need to move the DAL to another assembly, that would indeed help. –  Bob Horn Aug 19 '12 at 0:42
    
@BobHorn, so i don't get it. What's the problem to make your dal classes as internals? You can even make your DAL interfaces as internals. I don't see problems here. –  devundef Aug 19 '12 at 0:45
    
Having the DAL classes be internal is what we currently have, and what we want to have going forward. The interfaces will need to be public so the BLL can ask a factory for a concrete implementation, and not care if it's the real DAL or a mock. I guess my whole question is really a sanity check. Am I missing a better way to do this? –  Bob Horn Aug 19 '12 at 0:51

You seem immoveable on your ideas that you expounded in your question, but I'll tender an answer anyway - someone may find it useful sometime in the future.

If you insist on keeping your DAL internal but still want to hit it with some unit tests and don't mind using the Microsoft Unit Testing tools, then try the PrivateObject approach. This is a wrapper for reflection that you could always code yourself, but it saves you a few lines of typing.

Note that using InternalsVisibleTo is a viable approach, but it's clumsy - if you are using it then you may already be on a fast slide downhill. internal means something is public only within the bounds of it's assembly, that modifier is normally used because you don't want stuff useable from outside that assembly, then you promptly turn around and violate that concept by declaring a InternalsVisibleTo... that is a big dirty red flag that is screaming out for some refactoring. Decoupling is a waste of time when only one assembly is calling the other, you may as well glue them together. Decoupling infers that multiple assemblies are calling the target (introducing unit testing doesn't count towards the 'multiple' because there are ways to get to the target assembly like I've already mentioned).

share|improve this answer
    
Why do I seem immovable on my ideas? I'm willing to agree with you; I just need a discussion to convince me first. And don't tell me I violate the concept by using InternalsVisibleTo. I'm not using it. For some reason you're assuming I'm using it. I don't get why. –  Bob Horn Aug 19 '12 at 14:13
    
By the way, I appreciate your views because it's causing me to reconsider mine. Remember though, I'm allowed to have my opinion too. No need to get snarky because I don't immediately agree with you. –  Bob Horn Aug 19 '12 at 14:17
    
@Bob, sorry I got hooked up on that particular angle, I see from your comment 'and if I need to move the DAL to another assembly' that your DAL is probably right where I would have suggested it should be for an internal item. When you've decided what to do make sure you write it up here (if no answers fit), it'll be interesting to see what you eventually come up with. –  slugster Aug 19 '12 at 20:20

The interface approach is a good / mainstream one for mocking out your DAL during testing of consumers like the BLL.

One thing to note however is that by making your DAL concrete classes internal, is that you may be making your concrete DAL harder to test directly, as this now implies that your DAL unit tests must also be in this assembly without needing to use back doors like reflection.

Also, at some point in future you may consider using an IoC container to build up the implementation behind an interface, which also might trip up on the internal.

You could mimic the encapsulation you are trying to achieve with 'internal only' by adding a convention / LINT rule that your BLL / other layers must couple to the DAL via the interface, not on the concrete implementation.

share|improve this answer
    
Thanks, but the Visual Studio unit tests can actually test internal and private methods in other assemblies. –  Bob Horn Aug 20 '12 at 15:50

Technically, there is nothing wrong with your design.

However, I would consider an alternative approach: instead of decoupling your BLL from your DAL, better decouple your DAL from your database. Allow creation of DAL objects in-memory, and if your BLL needs to load DAL objects from somewhere, use the repository pattern (see here) to avoid direct contact of the BLL to the database. That way, you can

  • create unit tests for you DAL objects easily without the need of a database
  • create unit tests for your BLL by providing in-memory created DAL objects through a mock repository as test data (ok, you can argue now if those tests should be really called unit-tests)

In fact, what you loose is the capability of testing the BLL with DAL mocks. But in reality, those DAL mock objects will typically look a lot like your real DAL objects to provide a base for useful tests of the BLL. IMHO it avoids a lot of code duplication and unnecessary effort when re-using your real DAL objects for automated tests of the BLL.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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