p2p.wrox.com Forums

p2p.wrox.com Forums (http://p2p.wrox.com/index.php)
-   ADO.NET (http://p2p.wrox.com/forumdisplay.php?f=109)
-   -   Please help me to improve this ADO.NET code. (http://p2p.wrox.com/showthread.php?t=69485)

edurazee July 20th, 2008 05:12 AM

Please help me to improve this ADO.NET code.
 
Quote:

quote:This is the code which is intended to be used for connection to, and, manipulating database content.
Please try to improve the code.
Keep the core structural concept unchanged.
Code:

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

namespace MyDatabaseManupulationNamespace
{
    // static clss for manipulating Database
    public static class Database
    {
        private static string myConnectionString;
        private static string mySelectQueryString;
        private static string myGeneralQueryString;
        private static string myHost;
        private static string myDatabase;
        private static string myUser;
        private static string myPassword;
        private static string myTableName;

        private static SqlConnection myConnection;
        private static SqlCommand myCommand;
        private static SqlDataAdapter myDataAdapter;
        private static DataSet myDataSet;

        private static int rows = 0;

        private static bool successful = true;

        // property for setting new query
        public static string Query
        {
            set
            {
                myGeneralQueryString = value;
            }
            get
            {
                return myGeneralQueryString;
            }
        }

        // number of affected rows
        public static int AffectedRows
        {
            get
            {
                return rows;
            }
        }

        // gets the DataSet
        public static DataSet DataSetValue
        {
            get
            {
                return myDataSet;
            }
        }

        // gets the DataAdapter
        public static SqlDataAdapter DataAdapter
        {
            get
            {
                return myDataAdapter;
            }
        }

        // static constructor for initializing
        // static attributes
        static Database()
        {
            mySelectQueryString = @"SELECT * FROM ";

            myConnection = new SqlConnection();
            myCommand = new SqlCommand();
            myDataAdapter = new SqlDataAdapter();

            myDataSet = new DataSet();
        }

        // Methosd for establishing connection
        public static bool EstablishConnection(string host, string database, string user, string password, string table)
        {
            myHost = host;
            myDatabase = database;
            myUser = user;
            myPassword = password;
            myTableName = table;

            myConnectionString = "Data Source=" + myHost + ";Initial Catalog=" + myDatabase + ";User ID=" + myUser + ";Password=" + myPassword + ";Integrated Security=True";

            mySelectQueryString += myTableName;

            myGeneralQueryString = mySelectQueryString;

            try
            {
                myConnection.ConnectionString = myConnectionString;
                myConnection.Open();

                myCommand.Connection = myConnection;
                myCommand.CommandText = myGeneralQueryString;

                myDataAdapter.SelectCommand = myCommand;

                rows = myDataAdapter.Fill(myDataSet, myTableName);

                successful = true;
            }
            catch
            {
                successful = false;

                throw new DatabaseException("Connection Failed!");
            }
            finally
            {
                myConnection.Close();
            }

            return successful;
        }

        // Method for restoring select query
        public static void RestoreDefaultQuery()
        {
            myGeneralQueryString = mySelectQueryString;

            Console.WriteLine(myGeneralQueryString);
        }

        // Method for executing query
        public static bool ExecuteQuery()
        {
            try
            {
                myConnection.Open();

                myCommand.CommandText = myGeneralQueryString;

                myDataSet.Reset();

                rows = myDataAdapter.Fill(myDataSet, myTableName);

                successful = true;
            }
            catch
            {
                successful = false;

                throw new DatabaseException("SQL Query Failed!");
            }
            finally
            {
                myConnection.Close();
            }

            return successful;
        }       
    }
}


Imar July 20th, 2008 05:17 AM

Quote:

quote:Please try to improve the code.
Keep the core structural concept unchanged
Heuh? What's this? A homework assignment? Shouldn't you do this yourself?

If it's not homework, please elaborate on why you think the code needs to be improved, and in what areas you think it should be improved.

Imar


---------------------------------------
Imar Spaanjaars
http://Imar.Spaanjaars.Com
Everyone is unique, except for me.
Author of Beginning ASP.NET 3.5 : in C# and VB, ASP.NET 2.0 Instant Results and Dreamweaver MX 2004
Want to be my colleague? Then check out this post.

edurazee July 21st, 2008 09:49 PM

Quote:

quote:This is no homework assignment. I am just trying to understand how experienced programmers think and solve problems.

Please make it useful for manipulating Sql Server databases.

Imar July 22nd, 2008 02:44 AM

Again, it would help if you provided more information.....

Where does this code come from? What parts do you and don't you understand? Where do you see possible problems?

AFAICS, it's already working with SQL Server databases, considering the SqlConnection and other classes.

However, IMO, it has a few design flaws. First of all, it uses embedded connection strings. IMO, you should put them in an app.config file.

Secondly, the entire class is static. This will produce great problems.

For example, consider the Query property. This is shared by *all users* of this class. So, one user sets the query, then tries to execute it. However, before the query is executed, a second user overwrites the Query property. This gives the first user the results of the query of the second user. Not really reliable for a data access class... ;)

So, all in all, I don't think this is great code. You may want to pick up a book like Beginning Databases or look at the Microsoft Quick Starts for ADO.NET. For example, look here: http://www.asp101.com/articles/jay/a...et/default.asp
However, this is just an example. Searching Google for Microsoft Quick Starts for ADO.NET should yield many more useful results.

Hope this helps; if you need more information, please ask specific questions. "Please try to improve the code. Keep the core structural concept unchanged." is usually considered an assignment, not a great or useful question.

Cheers,

Imar

---------------------------------------
Imar Spaanjaars
http://Imar.Spaanjaars.Com
Everyone is unique, except for me.
Author of Beginning ASP.NET 3.5 : in C# and VB, ASP.NET 2.0 Instant Results and Dreamweaver MX 2004
Want to be my colleague? Then check out this post.

edurazee July 22nd, 2008 06:52 AM

I wrote this code for myself only for learning purposes.

I made the class "static" thinking that, one DB application don't need to have any instance object for DB - manipulation class. Because, in most cases there is only one RDBMS system it is working with, isn't it?

I know that this code is not in a standard to be used in an enterprise solution. So I need to know what a professional ADO.NET programmer will write in this case.

Please tell me what more sections should be changed or eliminated.

That's it!


Imar July 22nd, 2008 08:12 AM

It makes perfect sense to make the class partially static. However, you then need to pass user data directly in a method. For example:

public static DataSet GetDataSet(string sql)
{
  // Create *new* SqlConnection
  // Execute connection
  // return DataSet.
}

This way, the method is easy to use:

DataSet myDataSet = DataAccess.GetDataSet("SELECT Id, Name FROM table");

However, since inside the method a new connection is created, opened and closed, each user can work with their own data and queries.

Hope this helps,

Imar

---------------------------------------
Imar Spaanjaars
http://Imar.Spaanjaars.Com
Everyone is unique, except for me.
Author of Beginning ASP.NET 3.5 : in C# and VB, ASP.NET 2.0 Instant Results and Dreamweaver MX 2004
Want to be my colleague? Then check out this post.

edurazee July 22nd, 2008 09:55 PM

Thanks for the help!

I understood the design flaw.



All times are GMT -4. The time now is 01:42 PM.

Powered by vBulletin®
Copyright ©2000 - 2020, Jelsoft Enterprises Ltd.
© 2013 John Wiley & Sons, Inc.