How to simplify this code?

Question

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 3-Aug-11 1:46am

Alpman

Add a Solution

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

5 solutions

Solution 5

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

Posted 3-Aug-11 10:26am

Sergey Alexandrovich Kryukov

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.

Solution 3

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.

Posted 3-Aug-11 2:16am

Not Active

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.

Solution 2

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;
...

Posted 3-Aug-11 2:01am

#realJSOP

Updated 3-Aug-11 2:12am

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.

Solution 1

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();	
		}
	}
}

Posted 3-Aug-11 1:59am

Herman<T>.Instance

Updated 3-Aug-11 2:19am

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

Add a Solution

Add your solution here

Treat my content as plain text, not as HTML

Preview 0

…

Existing Members

Sign in to your account

...or Join us

Download, Vote, Comment, Publish.

Your Email
Password
Forgot your password?

Your Email
This email is in use. Do you need your password?
Optional Password

I have read and agree to the Terms of Service and Privacy Policy
Please subscribe me to the CodeProject newsletters

When answering a question please:

Read the question carefully.
Understand that English isn't everyone's first language so be lenient of bad spelling and grammar.
If a question is poorly phrased then either ask for clarification, ignore it, or edit the question and fix the problem. Insults are not welcome.
Don't tell someone to read the manual. Chances are they have and don't get it. Provide an answer or move on to the next question.

Let's work to help developers, not make them feel stupid.

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

nedel · Accepted Answer · 2011-08-03T03:48:00

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();
}

How to simplify this code?

5 solutions

Solution 5

Solution 4

Solution 3

Solution 2

Solution 1

Add your solution here

Preview 0

Existing Members

...or Join us