[Resolved] Please evaluate C# code - Beginner level

I have recently, with the help of Fishcake and Sevenhalo, figured out how to connect to a SQL Server and retrieve some data.

In order to expand my knowledge, my goal is to write a class that will encapsulate this code. I understand that writing a class for my tiny website may be a bit excessive, but its more for the knowledge of "how-to" than practicality.

The class that I have written does somewhat work:

Code:

---

using System;
using System.Data;
using System.Data.SqlClient;
using System.Data.Sql;

public class Database
{
       
        private String DatabaseConnectionString = "Data source=*;" +
        "Database=*;" +
        "uid=*;" +
        "pwd=*";
       
        private SqlConnection objConnection;
        private SqlCommand objCommand;
        public SqlDataReader objReader;
        public String dbError;
       
       
       
        public String GetConnectionString()
        {
                return DatabaseConnectionString;
        }
       
        private SqlConnection CreateConnection()
        {
                SqlConnection objConnection = new SqlConnection(DatabaseConnectionString);
                return objConnection;
        }
       
        private SqlCommand CreateCommand(String p_Command, SqlConnection p_Connection)
        {
                SqlCommand objCommand = new SqlCommand(p_Command, p_Connection);
                return objCommand;
        }
       
        //Article Related
        public void GetArticleByID(String p_CategoryID, String p_ArticleID)
        {
                objConnection = CreateConnection();
                objCommand = CreateCommand("SELECT * FROM tblArticle " +
                "WHERE tblArticle.charArticleID=@Article " +
                "AND tblArticle.charCategoryID=@Category", objConnection);
               
                SqlDataReader objReader;
               
                try
                {
                        objConnection.Open();
                        objCommand.Parameters.AddWithValue("@Article",p_ArticleID);
                        objCommand.Parameters.AddWithValue("@Category",p_CategoryID);
                       
                        objReader = objCommand.ExecuteReader();
       
                       
                }
                catch(System.Exception ex)
                {
                        dbError = ex.Message;
                        objReader= null;
                }
               
                       
        }
       
        //End Articles
}

---

The class is used like this:

Code:

---

<script runat="server" language="c#">
   
 
   
    protected void Page_Load(Object sender, EventArgs e)
    {
                String strCategoryID = Request.QueryString["ci"];
                String strArticleID = Request.QueryString["ai"];
               
               
                Database objDatabase = new Database();
                objDatabase.GetArticleByID(strCategoryID,strArticleID);
               
                if(objDatabase.objReader != null)
                {
                        while(objDatabase.objReader.Read());
                        {
                                label1.Text += objDatabase.objReader["charTitle"].ToString();
                        }
                }
                else
                        label1.Text = objDatabase.dbError;
               
        }
       
</script>

---

Question 1: Is my method even correct? Am I taking the right approach to this?

I'm trying to create shortcuts by making functions that create the connectionstring and command for me without me having to type it out each time. The idea behind the class is that, it will do all the work for me and store a SqlDataReader object in itself, that I will then manipulate in my Page_Load event.


Any input in regards to this question is greatly appreciated. Thanks!

What you've created is called a DAL - Data Access Layer. It's a class which encapsulates the database-specific methods. You're doing fine. Your approach is right, but the datareader isn't required here. In addition, your GetArticleById method is returning void. So, you need to make it return a dataset, and change its function signature to return a dataset.

In the code that calls this method, assign a dataset to the method, and use that dataset to... whatever.

when is it best to use the DataAdapter/DataSet approach and when is it best to use a DataReader?

I mean the DataAdapter/DataSet just seems like the long way instead of just using a DataReader.

It depends on what you need.... if you need to get some records, manipulate the data in memory and then send the changes back, a DataSet/Table would be the candidate. But if you are doing something like filling a combobox, then a Reader is ideal. Any time you need to loop through the data quickly and use it for something other than modifying the data, odds are, a reader will suffice. They come at a small price: 1) they create an exclusive lock on the connection, meaning you cannot use it for anything else. If you need to connect to do another db operation, a second connection will be needed. 2) They are read-only and forward only. You can't change the data and cannot move backwards in the data. But that's also where thier power comes from. By being what's called a Firehose cursor, it is very fast and efficient.

-tg

awesome... that is great to know, thanks tg!

I'll add to that. In a situation when you need to work with and update a very, very, very large set of data, do not use a dataset. Instead, use a datareader, and use separate code for the database manipulation.

Mendhak & TechGnome,

Thanks so much for the reply. Your opinions are appreciated!

Quote:

---

In addition, your GetArticleById method is returning void.

---

This is true. I actually changed the code without making the correct change in my question. Instead of returning something, I attached the Datareader to the object itself.

Instead of doing:
DataReader MyObject = GetArticleByID(X,Y)

I am manipulating the DataReader as part of the object:
objDatabase.DataReader

I think this is the best way to do it. I will update my initial post.

Quote:

---

but the datareader isn't required here

---

I guess I'm a little confused about what TechGnome said.

Quote:

---

t depends on what you need.... if you need to get some records, manipulate the data in memory and then send the changes back, a DataSet/Table would be the candidate. But if you are doing something like filling a combobox, then a Reader is ideal.

---

Since in the example I am only getting one article, its title, and the picture that is associated with it, would a DataReader not be the best? I only need a read only connection that retrieves data, I do not manipulate it in any way.

It seems to me that the dataset would be more appropriate for functions that modify the data such as:
AddNewArticle(X,Y);
EditArticle(X,Y);

Thanks again for the replies.

Actualy if all you are returning is a single record.... you should look into using output parameters and use the ExecuteNonQuery method.

-tg

Quote:

---

Originally Posted by invitro

Since in the example I am only getting one article, its title, and the picture that is associated with it, would a DataReader not be the best?

---

Nope. A dataset would be ideal here. You have a chunk of text, a string, and an image. You are not moving anywhere within the data. So, you need a dataset.

Have your method GetArticleById() return a dataset.

Got it, thanks for the replies all. I will use a DataSet!