Wrapping a .NET Database Connection

I have created a DatabaseConnection class as this:

class DatabaseConnection
{
    public SqlConnection sqlconn { get; set; }
    public DatabaseConnection(string server, string db, string user, string pass)
    {
        sqlconn = new SqlConnection(@"Data Source=" + server + ";Initial Catalog=" + db + ";User ID=" + user + ";Password=" + pass);
        OpenConn();
    }

    public SqlConnection OpenConn()
    {
        try{
            if (sqlconn != null && sqlconn.State == ConnectionState.Closed)
                sqlconn.Open(); // MessageBox.Show("Connection has been successfully established");
        }

        catch(Exception ex){
            MessageBox.Show(ex.Message);
        }

        return sqlconn;
    }
}

Its only purpose is to open the SqlConnection. I am just following the Single Responsibility Principle.

And then, in every class that needs a DatabaseConnection, this is my approach:

class DataViewer
{
    private DatabaseConnection dbCon;
    private SqlConnection sqlconn;
    private SqlDataAdapter sqlDA;

    //Here I am applying constructor injection
    public DataViewer(DatabaseConnection dbCon)
    {
        this.dbCon = dbCon;
        this.sqlconn = dbCon.sqlconn;
    }
    public DataGridView getDataInStoredProc(DataGridView dtgrdView, string storedProcName, params SqlParameter[] parameters)
    {
        try{
            //Some logic here
            sqlconn.Close();
        }

        catch (Exception ex){
            MessageBox.Show(ex.Message);
            sqlconn.Close();
        }
        return dtgrdView;
    }
}

Now, I would like to know if I am doing good or bad approach for this. If not, could you please explain me why and give me some advice on how to do it right?

I see a few issues with your class:

Issue 1

This property is open so anyone can set it:

public SqlConnection sqlconn { get; set; }

You probably want to make the setter private so only you can set it from within the class and it can be accessed from outside.

Issue 2

The property name should be SqlCon because the convention is to use Pascal naming in C#.

Issue 3

You have this in your constructor: OpenConn(); which means as soon as someone creates an instance of your class, you open the connection but what if the user of your class does not want to open the connection yet?

Issue 4

Why are you making the assumption that the user of you class will be using windows forms? What if I use your class from a console application, how will MessageBox.Show work?

catch(Exception ex){
    MessageBox.Show(ex.Message);
}

Issue 5

What if the user of your class forgets to close the connection?

Issue 6

What are you getting from this class? I will not use your class because the one in .NET is much better and easier to use. I can use it like this:

using (SqlConnection connection = new SqlConnection(connectionString))
{
    connection.Open();
    // Do my work
}

So if I am on your team and you tell me to use your class, which is a wrapper, first question I would ask you is: Why? And then if I find out that as soon as I create an instance of your class, you are opening database connections, I will not be very happy.

Patterns

Patterns such as Single Responsibility Pattern (actually it's a principle but beyond the point) and many other patterns that are there should be used by programmers who have a black belt, or at least a few belts, in programming. If someone is learning programming, they should not even bother with patterns. Patterns are the cool thing and with names like Singleton, Liskov Substitution Principle, Memento Pattern and other fancy names, I have to admit they are so tempting. People throw these terms around because they want to sound smart but do not give into this temptation. People want to use them instead of just creating a plain solid class.

Many people on StackOverflow ask questions such as "Why do people give so much attention to data structures, will I ever need to create one?" My answer to this question is simple: Yes, you will create them every single time you create a class. Just because you are creating a class called DatabaseConnection or Employee does not mean they are not data structures. They are. They are encapsulating data and you better make sure no one puts the state of your class into a bad state: like you have the sqlconn property public.

Forget Patterns for now

Do not worry about patterns for now. Just study data structures and learn how much effort is put into making the data structure so it is never in a bad state. Pay attention to how the operations are defined so users can interact with the data, add data, manipulate data, delete data and what it does when someone tries something they should not be allowed. In other words, when someone tries something the are not allowed does the data structure show a message box? Does it write to the console? Does it write to a file?

At the end of the day, in C# that is and many other OO languages, data structures are just good solid classes. If you know how to make your own ArrayList class, which is an array that can grow and shrink, and you can make it in a way that the class will never be in a bad state, regardless of how irresponsible or clever the programmer who is using your class is, then you will know how to make good solid classes.

Then you can start looking at patterns.

I am sorry if I sound harsh in any way, it is just because I want so speak plainly and I think this is the best way to get the message across.

StackExchange Code Review Q#153344, answer score: 4