3

I have many algorithm classes that implement the same interface; another "factory" class has the responsibility to instantiate the correct algorithm class through a config param, then call the start method on that instance.

I would like to restrict the the visibility of the constructor (or any other instance creation mechanism) of the algorithm classes to the factory class only.

How can I solve this? The only clean solution I can think about is to move those classes in a different .dll and change algorithm classes to private, but that's not what I want to do right now.

dav_i
  • 27,509
  • 17
  • 104
  • 136
SteBert
  • 145
  • 7
  • 4
    [internal (C# Reference)](http://msdn.microsoft.com/en-us/library/7c5ka91b.aspx) if i get you right thats what you looking for – Mark Sep 11 '14 at 08:00
  • 4
    If factory and algorithms are in the same DLL just make their constructors `internal` (of course everything inside that DLL will be able to create an instance of them without using factory). In C# there isn't the concept of `friend` like C++ so you can't make private stuff visible to a selected class/function. If you want to be sure _you_ won't forget to call factory method (even within same DLL) then you can make constructor `private` and declare a factory `internal` method for each class (itself is useless but it'll help you to prevent _mistakes_ because it's more _explicit_). – Adriano Repetti Sep 11 '14 at 08:01
  • 1
    You could make it so the constructors of the algoritms must take in a factory as a parameter, whilst this won't hide the visibility, you can make it cause an error whenever someone tries to use it – Sayse Sep 11 '14 at 08:11
  • 1
    If you want your constructor to be public, then you can check in your constructor who is making the call, if its from the factory class continue; else throw exception;. But it will cost performance due to the use of reflection. – Savaratkar Sep 11 '14 at 08:18

2 Answers2

3

This will not be an answer you will like to hear, but: don't.

By making that constructor private/internal/protected, you are making it hard to test the algorithms, and you are also preventing any consumers of the API from manually choosing their own implementation instead of using the factory.

I would say leave the constructors public, just make sure every dependency required is declared in the constructor. Here's my take on the various modifiers on constructors:

  • public - everyone can access your constructor, and you can test the class - this is good.
  • protected - subclasses can access the constructor. This is OK, but is only really used for abstract classes and/or constructor chaining.
  • internal - You're restricting the usage of the constructor to InternalsVisibleTo (friend) assemblies. This means that anyone within your assembly can construct it normally, but other assemblies have to explicitly be either InternalsVisibleTo or be forced to use your factory, thereby coupling the algorithm to the factory.
  • private - Useful for hiding a default constructor (although you don't need to do this if you create a constructor with at least one argument), or creating chainable constructors only.

TL;DR - I would recommend against making the constructor internal. By doing so you might as well make the class internal (you are referencing the algorithms by their interface and not their class, right?)

Another solution would be to make the implementations of the classes private and nest them inside of the Factory. Have them all implement a common interface (which they should be doing already) and expose the interface to the rest of the application. This still does not avoid the problem of making it hard to test.

Dan
  • 10,282
  • 2
  • 37
  • 64
  • We don't know much about his requirements, he may need to implement an interface for each class but it's viable. That said it's the same to have an internal constructor. Is it bad for tests? No, user will see an object created through a factory method then that's what you have to test. Do you need to make a class internal if constructor is internal? No, they're distinct things... – Adriano Repetti Sep 11 '14 at 09:06
  • By relying on the factory you are no longer testing the algorithm, you are testing the factory *and* the algorithm. That's fine for integration testing but generally algorithms need some form of unit testing. That is why I suggest making the algorithms have public constructors and testing them individually. – Dan Sep 11 '14 at 09:07
  • Yes, class and constructor internal *are* different. But there is no need to access an object through it's implementation (class) if you cannot create it (via the constructor). That's why I said he is likely better off setting the *class* to internal and not the constructor, if that is the only constructor on the class. The only reason otherwise to keep the class public would be to do something like `var myAlgorithm = (MyAlgorithmImplementation)factory.GetAlgorithm();` where `factory.GetAlgorithm()` returns `IAlgorithm` – Dan Sep 11 '14 at 09:08
  • IMO even if you don't have a public constructor you still may want to have a public class. For example when object creation is internal to a class (not only because of a factory method) but its usage is external. Even trough a factory method GetAlgorithm() doesn't need to be typeless: if you have GetAlgorithm() then T can be exactly returned type (not a base class or a most derived one) but object is initialized somehow and you want to keep client unaware of such details. Use case list is pretty endless... – Adriano Repetti Sep 11 '14 at 09:13
  • I just don't see the use case here. That said I would definitely have an interface backing this and I would definitely want to unit test the algorithms which is why I have made my suggestion. I am sure you are making yours because you have used the given use case before, so I will not say that your use case is wrong :-) – Dan Sep 11 '14 at 09:16
  • For sure depends on what you're doing! Let me do an example: you have a `sealed class Person {}` class. You don't want to make its constructor public because 1) when queried at least `Id` must come from DB and it must be initialized. 2) When you create a new `Person` object you want to fill its properties with defaults that come from DB/Configuration (so users can't customize default values). You _may_ put all this logic in constructor (but you want to keep who calls it unaware of DB connections and stuff like that) then you use a factory method (with `internal` constructor). – Adriano Repetti Sep 11 '14 at 09:22
  • It'll be easier to test too because factory method can be _mocked_ with DI. `Person` won't be aware of such details (keep it stupid) and test will be easier (moreover you won't test only `Person` but also its factory because users will create it through that method then it must be tested too. Even without factory method but with `internal` constructor: imagine your class uses `internal` classes and you pass them through constructor: constructor cannot be `public` unless you make `public` that classes too (but you want to completely hide such implementation details): let's make ctor `internal`! – Adriano Repetti Sep 11 '14 at 09:24
  • About interfaces: I use interfaces if _required_ (.NET is not COM) but I try to avoid them when they're 1:1 with classes (`ICat` and `Cat`, `IPerson` and `Person`, `IOrder` and `Order`, what's for? Just to keep ctor visibility in sync with class visibility? Why?). Anyway I didn't downvote even if I completely disagree because I think we're talking about points of view! – Adriano Repetti Sep 11 '14 at 09:30
2

Maybe you will be satisfied by the following sample (yes, I know what reflection shouldn't be used normally...)

public class Algorithm
{
    private Algorithm()
    {
    }

    public void SomeMethod()
    {
    }
}

public static class Factory
{
    public static Algorithm Create()
    {
        var constructor = typeof(Algorithm).GetConstructor(BindingFlags.NonPublic | BindingFlags.Instance, null, new Type[0], null);
        return (Algorithm)constructor.Invoke(null);
    }
}

Here you can create instance of Algorithm via Factory.Create, not by new Algorithm.

Andrey Korneyev
  • 26,353
  • 15
  • 70
  • 71