Click here to Skip to main content
15,891,033 members
Please Sign up or sign in to vote.
0.00/5 (No votes)
See more:
Hello,

I'm quite new to C# and I like to know how to simplify the following code. Its purpose is to query tables in an Access MDB. Since the queried databases can obtain different tables I created two enumerators for the table names.

C#
public enum TablesGeometriesMDB
        {
            chain,
            link,
            link_coords,
            node,
            point,
            simple_area,
            simple_area_coords,
            simple_chain,
            simple_chain_coords,
            simple_point,
            text
        }

public enum TablesDesignObjectsMDB
        {
            drawings,
            projects
        }


In each case of my switch I'm using a foreach-loop to get the enumerated values and execute the queries. The enumerator is dependent on the name of database.

...
OleDbConnection con = new OleDbConnection(...):
con.Open();

switch(strSelectedMDB) {
        case "geometries":
                            
            foreach (TablesGeometriesMDB table in Enum.GetValues(typeof(TablesGeometriesMDB)))
            {
                strSQL = "SELECT COUNT(*) FROM [" + table + "]";
                OleDbCommand cmd = new OleDbCommand(strSQL, con);
                OleDbDataReader dr = cmd.ExecuteReader();
                while (dr.Read())
                {
                    lstOutput.Items.Add(table + ": " + dr[0].ToString());
                    lstOutput.ScrollIntoView(lstOutput.Items.GetItemAtlstOutput.Items.Count - 1));
                    ICountMDBObjects += Convert.ToInt32(dr[0]);
                }
                dr.Close();
             }
             break;
         case "objects":
             foreach (TablesDesignObjectsMDB table in Enum.GetValues(typeof(TablesDesignObjectsMDB)))
             {
                 strSQL = "SELECT COUNT(*) FROM [" + table + "]";
                 OleDbCommand cmd = new OleDbCommand(strSQL, con);
                 OleDbDataReader dr = cmd.ExecuteReader();
                 while (dr.Read())
                 {
                     lstOutput.Items.Add(table + ": " + dr[0].ToString());
                     lstOutput.ScrollIntoView(lstOutput.Items.GetItemAtlstOutput.Items.Count - 1));
                     ICountMDBObjects += Convert.ToInt32(dr[0]);
                 }
                 dr.Close();
              }
              break;
...

How can I simplify this code? I can of course put the body of the foreach-loop in a function. But can I write a function that takes the enumerator as a variable so that the whole loop can be put in this function?
Posted
Comments
Herman<T>.Instance 3-Aug-11 7:49am    
well...each foreach loop in it's own method is surely advisable. It makes the code more readable.
Alpman 3-Aug-11 8:10am    
Thanks for your and Johns work. So there is no way to write and call a function like this

case "geometries" : DoIt(TablesGeometriesMDB)

case "objects" : DoIt(TablesObjectsMDB)

public void DoIt(xxx) { foreach( table in xxx) ... } ?
Herman<T>.Instance 3-Aug-11 8:17am    
both objects are enums so a typeof(object) cannot be done
Herman<T>.Instance 3-Aug-11 7:51am    
this part:
OleDbDataReader dr = cmd.ExecuteReader();
while (dr.Read())
{
lstOutput.Items.Add(table + ": " + dr[0].ToString());
lstOutput.ScrollIntoView(lstOutput.Items.GetItemAtlstOutput.Items.Count - 1));
ICountMDBObjects += Convert.ToInt32(dr[0]);
}
dr.Close();

could also be set to 1 method

Move the query code into a method:

C#
public Int CountRecords(string table)
{
    int count = 0;
    try
    {
        strSQL = "SELECT COUNT(*) FROM [" + table + "]";
        OleDbCommand cmd = new OleDbCommand(strSQL, con);
        OleDbDataReader dr = cmd.ExecuteReader();
        while (dr.Read())
        {
            lstOutput.Items.Add(table + ": " + dr[0].ToString());
            lstOutput.ScrollIntoView(lstOutput.Items.GetItemAtlstOutput.Items.Count - 1));
            count = Convert.ToInt32(dr[0]);
        }
    }
    catch (Exception ex)
    {
    }
    finally
    {
        if (dr != null)
        {
            dr.Close();
        }
    }
}

And then your existing code would look like this:

C#
OleDbConnection con = new OleDbConnection(...):
con.Open();
 
switch(strSelectedMDB) 
{
    case "geometries":
        foreach (TablesGeometriesMDB table in Enum.GetValues(typeof(TablesGeometriesMDB)))
        {
            ICountMDBObjects += CountRecords(table.ToString());
        }
        break;

    case "objects":
        foreach (TablesDesignObjectsMDB table in Enum.GetValues(typeof(TablesDesignObjectsMDB)))
        {
            ICountMDBObjects += CountRecords(table.ToString());
        }
        break;
...
 
Share this answer
 
v2
Comments
Herman<T>.Instance 3-Aug-11 8:12am    
strange switch case in your seconde code block
#realJSOP 3-Aug-11 8:22am    
Cut/paste error
Miroslav Bučko 16-Aug-11 16:57pm    
Where is the return value in CountRecords method?
It is not defined as void.
strange idea to keep the table names as a enumeration. Why not to use the string array like:
C#
string [] TablesGeometriesMDB = new string[]
        {
            "chain",
            "link",
            "link_coords",
            "node",
            "point",
            "simple_area",
            "simple_area_coords",
            "simple_chain",
            "simple_chain_coords",
            "simple_point",
            "text"
        };

string [] TablesDesignObjectsMDB = new string[]{"drawings", "projects"};
void GetData(string strSelectedMDB)
{
  switch(strSelectedMDB) {
	case "geometries":
          GetDataFromDB(TablesGeometriesMDB);
	  break;
	case "objects":
          GetDataFromDB(TablesDesignObjectsMDB);
	  break;
  }
}
void GetDataFromDB(string[] tables)
{
  OleDbConnection con = new OleDbConnection(...):
  con.Open();
  foreach (string table in tables)
        {
          strSQL = string.Format("SELECT COUNT(*) FROM [{0}]", table);
          OleDbCommand cmd = new OleDbCommand(strSQL, con);
          int cnt = Convert.ToInt32(cmd.ExecuteScalar());
          lstOutput.Items.Add(string.Format("{0}: ", table, cnt);
          ICountMDBObjects += cnt;
	}
  lstOutput.ScrollIntoView(lstOutput.Items.GetItemAtlstOutput.Items.Count - 1));
  con.Close();
  con.Dispose();
}
 
Share this answer
 
v2
Comments
Alpman 3-Aug-11 10:18am    
Lol, I guess that's what I tried to achieve all the time. How did I miss arrays? *slap self*
another suggestions

List<string> enumValues = new List<string>();
switch(strSelectedMDB) 
{
	case "geometries": DoGeoMetries(); break;
	case "objects": DoObjects(); break;
	default: break;
}	
AddToDataBase(enumValues); 
}
private void DoGeoMetries()
{
	enumValues = new List<string>();
	foreach (TablesGeometriesMDB table in Enum.GetValues(typeof(TablesGeometriesMDB)))
	{
		enumValues.Add(table.ToString());
	}
}
private void DoObjects()
{	
	enumValues = new List<string>();
	foreach (TablesDesignObjectsMDB table in Enum.GetValues(typeof(TablesDesignObjectsMDB)))
	{
		enumValues.Add(table.ToString());
	}
}
private void AddToDataBase(List<string> enumValues)
{
	foreach (string enumValue in enumValues)
	{
		OleDbConnection con = new OleDbConnection(...):
		string strSql = string.Format("SELECT COUNT(*) FROM [{0}]", enumValue);
		OleDbCommand cmd = new OleDbCommand(strSQL, con);
		try
		{
			con.Open();
			using (OleDbDataReader dr = cmd.ExecuteReader())
			{
				while (dr.Read())
				{
					 lstOutput.Items.Add(table + ": " + dr[0].ToString());
					 lstOutput.ScrollIntoView(lstOutput.Items.GetItemAtlstOutput.Items.Count - 1));
					 ICountMDBObjects += Convert.ToInt32(dr[0]);
				}
				dr.Close();
			}
		}
		catch(Exception)
		{
		}
		finally
		{
			if (con != null && con.State == ConnectionState.Open)
				con.Close();	
		}
	}
}
 
Share this answer
 
v3
Comments
Herman<T>.Instance 3-Aug-11 8:00am    
you do not have to use the last line. this is done when adding the source to the project and is not paret of the solution
thatraja 3-Aug-11 8:20am    
Removed the unwanted <string>s from last line
You could greatly simply this by creating one query with all selects and filling a DataSet, the iterate through the DataTables.

StringBuilder query = new StringBuilder();
query.AppendLine("SELECT [Fields] FROM table1 ");
query.AppendLine("SELECT [Fields] FROM table2 ");
query.AppendLine("SELECT [Fields] FROM table3 ");

string[] tableNames = new string[]{"Table1", "Table2", "Table3"};

DataSet ds = new DataSet();
using(SqlConnection conn = new SqlConnection(...))
{
  using(SqlCommand cmd = new SqlCommand(query.ToString(), conn))
  {
    conn.Open();
    ds.Load(cmd.ExecuteReader(), LoadOption.OverrightChanges, tableNames);
  }
}

foreach(DataTaable dt in ds.Tables)
{
  foreach(DataRow row in dt.Rows)
  {
    ....
  }
}


Also note you can use SqlExpress rather than Access. It is more robust than Access, IMO, and offers great capability to migrate upward when/if necessary, and it is free.
 
Share this answer
 
Comments
Alpman 3-Aug-11 8:21am    
Before I started using different enumerators I put all tables in one enumerator. But then I got an error message, that for example the table "drawing" could not be found in the database "geometries". Won't this message appear with your solution, too?
[no name] 3-Aug-11 8:50am    
Yes, of course if the connection strings are different you'll need to change it. However, this could easily be written into a method that accepts, connection string, query and table names.
The problem is simple: enum types do not support IEnumerable (!). I can offer a decent solution for enumeration based on enumeration types. Please see my article:
Enumeration Types do not Enumerate! Working around .NET and Language Limitations[^].

In this article you will also see an overview of a bunch of more basic techniques and issues.

—SA
 
Share this answer
 
Comments
Alpman 4-Aug-11 3:24am    
Thanks for the link, I'll have a look at it.
Daniel Gidman 4-Aug-11 23:20pm    
Enum to Dictionary @ ExtensionMethod.net

Thats an extension methoad that will convert an enumeration to a dictionary.

This content, along with any associated source code and files, is licensed under The Code Project Open License (CPOL)



CodeProject, 20 Bay Street, 11th Floor Toronto, Ontario, Canada M5J 2N8 +1 (416) 849-8900