1

My first post here so hello all :)

I'm using stairway pattern e.g. I have Assembly A (entry point - composition root) which depends on Assembly B (containing interfaces) and Assembly C which contains implementations of interfaces defined in Assembly B:

A --> B <-- C

I'm developing in C# and I'm using Castle Windsor for IoC.

I'm also utilizing Windsor's ability to register non public types (https://github.com/castleproject/Windsor/blob/master/docs/registering-components-by-conventions.md -> Registering Non Public Types). I found it useful as I can define classes in assembly C to be internal which means that no one would create direct dependency on C from A. This way both A and C only depend on Assembly B which contains only abstractions which sounds right to me.

I'm wondering though if this is a good design choice. As far as I'm aware only Windsor allows to register non public types and they also warn to not expose non public types, hence my question. Also, I don't think this considerations is limited to .NET world so I'll appreciate insight into this problem regardless of technology you use :)

Private Void
  • 11
  • 1
  • I've done the same with Autofac using modules in each assembly that has internal types. the main (entry) assembly is only aware of the modules it needs to register (ie: email module, etc.) – Bishoy Jun 22 '16 at 01:41

1 Answers1

1

Is there any reason to shield classes of C from accessing from the outside? This only makes sense when you have third parties use this assembly (i.e. you publish it as NuGet package for others to use). In case the only consumers of this library is inside your own team, hiding the internals only complicates things, because:

  • Testing becomes harder, because you will have to mark stuff with [InternalsVisibleTo].
  • Composition becomes harder, because you can reference your components directly from within your composition root. Referencing them directly is needed when you register them one by one, which is sometimes required, or you wish to new them up by hand instead of letting them auto-wire by the container.
  • Since the Composition Root (assembly A) is the only assembly who takes a dependency on assembly B, you are basically only hiding the dependencies from the composition root, which makes little sense, because by definition, the composition root depends on all assemblies in your application.

On the other hand, making classes internal makes a lot of sense from perspective of a reusable library. Take a look at the .NET framework itself and how much of it is internal. Making stuff internal makes it easier to change stuff without introducing breaking changes.

Steven
  • 166,672
  • 24
  • 332
  • 435
  • Main reason for keeping implementations in C internal is enforcing dependency of A on abstractions from B and avoiding anyone to introducing dependency on implementations from C. Also, let's say that I have 10 classes in C and half of them have extracted interfaces (internal to C) to facilitate testing - I don't want these to be exposed to A to: 1. avoid introducing unwanted dependencies of A on C 2. avoid wiring them by hand. So, all of classes and interfaces in C are internal and auto - wired by windsor using IncludeNonPublicTypes option. – Private Void Jun 22 '16 at 20:18
  • Until someone adds an InternalsVisibleTo attribute and everything is accessible again. Try using a tool like NsCop to manage namespace dependencies or write a unit test that prevents dependencies from one assembly to the other. Also, without code reviews and proper communication with the developers about why you have such rule, this rules will simply fail. Architectural rules will be broken because of time pressure and Conway's Law. – Steven Jun 22 '16 at 20:35
  • While I find code reviews invaluable, I don't think it's enough in bigger team so when I can, I prefer to enforce as much as possible. But maybe handling it this way (making classes internal) is wrong approach and I should rather go down the architecture rules enforced through automated tests path? Generally though, my initial question was if making classes in assembly C internal and rely on DI to resolve dependencies automagically is a design smell? – Private Void Jun 22 '16 at 22:22
  • @PrivateVoid: No, I think calling it a design smell would be too harsh. In case you wish to enforce as much as possible, take a look at tools like NDepend, because there is a lot that you can't enforce in code (and developers will work around your barriers any way). Tools like NDepend allow you to specify architectural rules in a (query like) DSL, which is really powerful. – Steven Jun 23 '16 at 08:35
  • Thanks a lot for your replies Steven, I really appreciate your insight :) – Private Void Jun 23 '16 at 21:43