Obtain values from SQL

Question

I have a simple app which is checking occurrences of values. If there is already occurrences which I am looking for, it display that. If not, it adds.

create table Strings
(
    StringID INT IDENTITY(1,1) NOT NULL PRIMARY KEY
    , StringValue VARCHAR(100) NOT NULL
)

create table Links
(
    LinkID INT IDENTITY(1,1) NOT NULL PRIMARY KEY
    ,Link VARCHAR(255) NOT NULL
)

CREATE TABLE Occurences
(
    OccurenceID INT IDENTITY(1,1) NOT NULL PRIMARY KEY
    ,StringID INT NOT NULL
    ,LinkID INT NOT NULL
)

Class:

private SqlConnection conn;


public void Open()
{
    this.conn = new SqlConnection();
    this.conn.ConnectionString = @"Server=PC\SQLEXPRESS;Database=example;User=sa;Pwd=example;";
    this.conn.Open();
}

public void Close()
{
    this.conn.Close();
}


public string Obtain(string code)
{
    SqlConnection sqlConnection = new SqlConnection();
    sqlConnection.ConnectionString = @"Server=PC\SQLEXPRESS;Database=example;User=sa;Pwd=example;";
    SqlCommand sqlCommand = new SqlCommand();
    sqlCommand.Connection = sqlConnection;
    sqlCommand.CommandText = code;
    sqlConnection.Open();
    SqlDataReader sqlDataReader = sqlCommand.ExecuteReader();
    sqlDataReader.Read();
    string result = sqlDataReader.GetValue(0).ToString();
    sqlConnection.Close();
    return result;
}


public void Insert(string StringID, string LinkID)
{
    SqlConnection sqlConnection = new SqlConnection();
    sqlConnection.ConnectionString = @"Server=PC\SQLEXPRESS;Database=example;User=sa;Pwd=example;";
    SqlCommand sqlCommand = new SqlCommand();
    sqlCommand.Connection = sqlConnection;
    sqlCommand.CommandText = string.Concat(new string[]
    {
        "INSERT INTO Occurences VALUES ('",
        StringID,
        "', '",
        LinkID,
        "')"
    });
    sqlConnection.Open();
    sqlCommand.ExecuteNonQuery();
    sqlConnection.Close();
}

Button click:

private void button1_Click(object sender, EventArgs e)
{

    try
    {
        sql.Open();
        StringID = sql.Obtain("SELECT StringID from dbo.Strings where StringValue='" + textBox1.Text + "'");
        LinkID = sql.Obtain("SELECT LinkID from dbo.Links where Link='" + textBox2.Text + "'");
        Occurence = sql.Obtain("SELECT OccurenceID from dbo.Occurences where StringID='" + StringID + "' and LinkID = '" + LinkID + "'");
        if (Convert.ToInt32(Occurence) > 0)
        {
            MessageBox.Show(Occurence);
        }
    }

    catch
    {
        sql.Insert(StringID, LinkID);
    }

    finally
    {
        sql.Close();



    }
}

Is this coded well? Should I change my catch to more specific? Should I do something like these?

If there is no StringValue in dbo.Strings then insert StringValue
If there is no Link in dbo.Links then insert Link

You should be using parameterized queries, not open and close three times to get 3 items, and you don't need a reader to get one value. — paparazzo
– paparazzo, Commented Aug 13, 2016 at 15:38
For the sql connection, shouldn't you be using "using" keyword that implicitly calls IDisposable or maybe use try..catch to explicitly call Dispose(). — Tolani
– Tolani, Commented Aug 15, 2016 at 8:36

Heslacher · Accepted Answer · 2016-09-12 06:05:24Z

Usually when we do something similar we use a repository (pattern). It allows us to encapsulate the entire interaction with the database so that we don't have to think about the queries again and again.

For example:

class MyDatabase
{
    public MyDatabase(string connectionString = null)
    {       
        // use the specified or the default connection string
        ConnectionString = connectionString ?? @"Server=PC\SQLEXPRESS;Database=example;User=sa;Pwd=example;";
    }

    public string ConnectionString { get; private set; }

    public int GetStringID(string stringValue)
    {
        // here comes the select and connection manintenance
    }
}

When working with database objects you should always use the using statement to free the resources. Always check the documentaiton if something implements the IDisposable interface.

Your actual code should look like this:

using(var sqlConnection = new SqlConnection(ConnectionString))
{
    // work with the connection where
}

There are few more types that are disposable too (i.e. the SqlCommand or the SqlDataReader).

With the repository your button1_Click could look like this:

private void button1_Click(object sender, EventArgs e)
{
    var db = new MyDatabase();
    StringID = db.GetStringID(textBox1.Text);
}

Naming

Even when working with the simplest projects you should always use meaningful names and button1 or textBox1 are really bad. I don't know what the button1 does but the text-box could be named stringValueTextBox or the textBox2 should be called linkTextBox etc.

Stack Exchange Network

Obtain values from SQL

1 Answer 1

You must log in to answer this question.

Hot Network Questions

Obtain values from SQL

1 Answer 1

You must log in to answer this question.

Related

Hot Network Questions