0
\$\begingroup\$

I'm changing the inner implementation of a project. In order to do so, I've created an interface that will be implemented in 2 different ways.

One of this ways is by class A, which is a wrapper for an outside class (class "B") from a .dll file (a product of other team).

Class B is a private a member of class A. Class B ctor takes an argument, which does not change during the run of my application. It needed to be created only once, and to stay available for class A everywhere in the code.

Class A is an implementation of interface. It use the capabilities of class B in order to implement it's functions, and obviously needed to be available also everywhere in the code.

I've concluded that:

Class A has to be:

  1. explicitly created once (hence, not static)
  2. only once, (because I need B to be created only once.)
  3. accessible from anywhere in the code
  4. have a member (B) that must initialized once, and only once (private member, needed only inside class A)

Concerning condition #4, ideally I would like to initialize B in the constructor.

A bit similar to singleton pattern, with the exception of wanting to initialize a member in the constructor.

I've come to a solution, and would like to get feedback.

public sealed class A
{
   private static SomeClass B = null; // the "must initialized only once" member
   // setting B:
   private void setB(int i)
   {
      B = new SomeClass(i);
   }
   // ctor
   public A(int i)
   {
      if(B == null)
         setB(i);
      else throw new Exception("An instance of A can be created only once");
   }     
}

The idea is to assign the class to static object at the beginning of the code. It's not very neat, but it compiles and seems to do the job just right. I don't use singleton since it won't be possible to initialize the member in the ctor. By the way, thread safety is not a concern here.

SomeClass needed only inside class A. SomeClass comes from dll, and class A kind of wrapping it. A does answering cross-cutting concern, it is an implementation of Interface used all over the code. I've tried to implement singleton, but as much as I tried it didn't answer my needs.

\$\endgroup\$

closed as off-topic by Simon Forsberg, Vogel612, 200_success Apr 29 '14 at 18:42

This question appears to be off-topic. The users who voted to close gave this specific reason:

  • "Questions must involve real code that you own or maintain. Questions seeking an explanation of someone else's code are off-topic. Pseudocode, hypothetical code, or stub code should be replaced by a concrete example." – Simon Forsberg, Vogel612, 200_success
If this question can be reworded to fit the rules in the help center, please edit the question.

  • 2
    \$\begingroup\$ Why not go with a real, full-blown Singleton implementation? Thread safety not being an issue now might become an issue later... Why does it need to be accessible from anywhere in the code? Is it addressing a cross-cutting concern? Have you considered any alternative approaches? \$\endgroup\$ – Mathieu Guindon Apr 29 '14 at 17:32
  • 3
    \$\begingroup\$ Welcome to Code Review! If possible, please add some more context to your question. The more you tell us about what your real code does and what the purpose of doing that is, the easier it will be for reviewers to give you better help. Why do you need a singleton in your code? What is it's real purpose? \$\endgroup\$ – Simon Forsberg Apr 29 '14 at 17:45
  • 3
    \$\begingroup\$ Simon is right, this is pretty much impossible to review as is. You would get much better (and more applicable) answers with real code and more context ;) \$\endgroup\$ – Mathieu Guindon Apr 29 '14 at 17:50
  • 2
    \$\begingroup\$ My way of handling this would be to simply never create an instance of the class directly, and let the same instance be injected to everything that needs it (through a MEF import in my case). It won't stop other programmers from misusing your code by newing up more instances, but that isn't really your problem. \$\endgroup\$ – Magus Apr 29 '14 at 17:57
  • 1
    \$\begingroup\$ You have the beginnings of a good question, but hypothetical code lacks sufficient detail to be reviewed. We have therefore closed this question as off-topic, but would love to see you ask another question with real identifiers and a real use case so that we can give it a proper review. \$\endgroup\$ – 200_success Apr 29 '14 at 18:58
1
\$\begingroup\$

If you need to wrap SomeClass, make your wrapper an abstraction:

public interface ISomeClassWrapper
{
    // expose the methods and properties you want to wrap
}

Then everywhere it's needed in the code, replace static method calls like this:

public void DoSomething()
{
    SomeClass.Xyz();
}

With something like this:

public class MyOtherClass
{
    private readonly ISomeClassWrapper _wrapper;

    public MyOtherClass(ISomeClassWrapper wrapper)
    {
        _wrapper = wrapper;
    }

    public void DoSomething()
    {
        _wrapper.Xyz();
    }
}

Now, if you're worried about how many instances of SomeClassWrapper (which implements ISomeClassWrapper but MyOtherClass doesn't need to know that), don't instantiate it. ever.

How do you ensure there's only 1 instance of it?

When you've replaced SomeClass.Xyz(); with _wrapper.Xyz();, you've done several things:

  • MyOtherClass is now decoupled from SomeClass, and even from SomeClassWrapper.
  • The ISomeClassWrapper dependency is now statically declared as such, so just by looking at MyOtherClass's constructor you know it needs an implementation for ISomeClassWrapper to carry out its tasks.
  • You can now provide a fake/mock implementation of ISomeClassWrapper, and write a test for DoSomething() without worrying about side-effects in code that you have no control over.
  • The code that's responsible for providing the ISomeClassWrapper instance/implementation, is also responsible for passing everyone that wants an ISomeClassWrapper instance, the same instance. Most IoC containers can do this very, very easily.
\$\endgroup\$
  • \$\begingroup\$ look promising, i'll try it and report back \$\endgroup\$ – sivan shani Apr 29 '14 at 18:33
  • \$\begingroup\$ Good! If you're going to use an IoC container, make sure no code depends in it, otherwise you'll fall into the Service Locator trap; also if you're taking the Dependency Injection route, make sure you go all the way about it; I'd recommend Mark Seemann's Dependency Injection in .net, an excellent read on the subject ;) \$\endgroup\$ – Mathieu Guindon Apr 29 '14 at 18:41
  • \$\begingroup\$ What class implements ISomeClassWrapper? You still need to create an instance of whatever it is to pass to MyOtherClass, which in and of itself means you can create more than one instance of it anyway. \$\endgroup\$ – Brandon Apr 29 '14 at 18:48
  • 1
    \$\begingroup\$ The hardest part about avoiding a service locator is how similar it looks to proper dependency injection in some cases. The primary reason I've started using MEF is that it offers an attributed model, which allows you to avoid resolving your dependencies in code altogether; the attributed properties and constructors are simply fulfilled as available. @Brandon: While more than one instance can be created, if injected, it won't be. If someone does make another instance in your project, punch them. \$\endgroup\$ – Magus Apr 29 '14 at 18:50
  • 1
    \$\begingroup\$ @Brandon the wrapper itself! And yes, nothing prevents newing up another instance somewhere else, except following the DI pattern and sticking to it (hence "going all the way about it") \$\endgroup\$ – Mathieu Guindon Apr 29 '14 at 18:50

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