Is it OK to put a database initialization call in a C# constructor? [closed]

Question

I've seen this is various codebases, and wanted to know if this generally frowned upon or not.

For example:

public class MyClass
{
   public int Id;

   public MyClass()
   {
      Id = new Database().GetIdFor(typeof(MyClass));
   }
}

Answer 1

There are several reasons this is not generally considered good design some of which like causing difficult unit testing and difficulty of handling errors have already been mentioned.

The main reason I would choose not to do so is that your object and the data access layer are now very tightly coupled which means that any use of that object outside of it original design requires significant rework. As an example what if you came across an instance where you needed to use that object without any values assigned for instance to persist a new instance of that class? you now either have to overload the constructor and then make sure all of your other logic handles this new case, or inherit and override.

If the object and the data access were decoupled then you could create an instance and then not hydrate it. Or if your have a different project that uses the same entities but uses a different persistence layer then the objects are reusable.

Having said that I have taken the easier path of coupling in projects in the past :)

Answer 2

Well.. I wouldn't. But then again my approach usually involves the class NOT being responsible for retrieving its own data.

Answer 3

It will also make it difficult to write unit tests for the class as you won't be able to force the class to use a Mock/Stub version of the db class. See here: http://en.wikipedia.org/wiki/Dependency_injection

Answer 4

You can use the disposable pattern if you refer to a DB connection:

public class MyClass : IDisposable
{
    private Database db;
    private int? _id;

    public MyClass()
    {
        db = new Database();
    }

    public int Id
    {
        get
        {
            if (_id == null) _id = db.GetIdFor(typeof(MyClass));
            return _id.Value;
        }
    }

    public void Dispose()
    {
        db.Close();
    }
}

Usage:

using (var x = new MyClass()) 
{
    /* ... */

} //closes DB by calling IDisposable.Dispose() when going out of "using" scope

Answer 5

Yea, you CAN do it, but it's not the best design, and error handling in constructors isn't as tidy as elsewhere.

Answer 6

The only problem I can think of with this approach is that any errors from the DB initialization will be propagated as exceptions from the constructor.

Answer 7

Why would anyone want to use a mock object/stub instead of the real thing? Would you agree that car manufacturers should use paperboard models for crashtests?