views:

68

answers:

4

I have DataProvider class (DAL) that requires "year" as parameter. It used like this:

using (var provider = new DataProvider(year))
{
   provider.SomeRepostitory.DoSomethingUsefull(); 
}

DataProvider constructor code deals with configuration - so it can throw exceptions. And exception throwable constructors are not recommended. So I added Init method and put all throwable code there:

var provider = new DataProvider();
provider.Init(year);

But now there are two lines of code instead of one and as provider created many times across the code, i put these two lines into fabric static method:

using (var provider = DataProvider.Create(year))
{
  ...
}

Is it ok or is there a better solution?

Thank you in advance!

+2  A: 

I think it is perfectly reasonable to have a factory method that does some initialization on an object. If you are not supposed to use the object without initialization, make the constructor private (or protected) so that only the static method within the class is able to call the constructor.

driis
A: 

You could have the Init method return this if you wanted to chain the calls together. Like:

var provider = new DataProvider().Init(year);

But if you have to call .Init(), then the factory method style may be more helpful.

EliThompson
A: 

Can you get away with lazy instantiation? Pass in the year when you create it but don't actually initialize it until it's used. This would move the exception to the first call rather than the construction but it would seem to meet your requirements.

Loren Pechtel
That means you have to catch potential exceptions every time you use the "worker" method, though, which is potentially quite an overhead. It also means the first call to the method isn't as predictable as the rest, which is counter to good API design,
Dan Puzey
+4  A: 

DataProvider constructor code deals with configuration - so it can throw exceptions. And exception throwable constructors are not recommended.

Why are they "not recommended"? Look at the Microsoft base class libraries and you'll see that near enough every single constructor checks the arguments passed to it and throws an argument exception of some sort if an argument is invalid. Why would you let someone instantiate an object in an invalid state without telling them? Fail early, fail hard.

A constructor throwing an exception is a much better API to program against than having to use a separate initialise method (I personally detest two-phase initialisation; it's so easy to forget the second call). Unless there's a compelling reason to avoid it, I would just do the checks in the constructor.

If you really want to use a static factory method (nothing wrong with this unless you need to make it loosely coupled), make sure the default constructor is private so users are forced to use the appropriate factory method.

Mark Simpson