current community

your communities

more stack exchange communities

Stack Exchange
tour help
Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 5 down vote favorite

Overview

I've written a class that will create C# code. The output is a single .cs file for each table in the default database (the database is defined inside a web.config file). I'm looking for a code review on this class alone, not on the generated code. This DatabaseClassCreator class uses the DatabaseAccess class for some of its database access. The DatabaseAccess class can be seen here as I'm asking for a code review of that class as well. If you are not interested in seeing the DatabaseAccess class, there is one static method ExecSQL, which returns a single DataTable with the results of the passed SQL.

Two notes:

  1. This was developed and is being used for a ASP.NET project, which is the reason for the BulletedList.
  2. This class has to work in a C# .NET 2 environment, so anything that's more modern would be interesting to me, but please note in your answer if the comments/feedback require a newer .NET version.

/// <summary>
/// This class will create c# class files to access spesific
/// tables in a database.
/// </summary>
public static class DatabaseClassCreator
{
    /// <summary>
    /// Create class files for all the non-system tables in the current
    /// default database.
    /// </summary>
    /// <param name="OutputPath">The output location for the class files. This
    /// is a fully qualified path.</param>
    public static void CreateAllTables(string OutputPath)
    {
        BulletedList bl = new BulletedList();
        CreateAllTables(bl, OutputPath);
    }
    /// <summary>
    /// Create class files for all the non-system tables in the current
    /// default database.
    /// </summary>
    /// <param name="StatusBulletList">A BulletedList where status information can be
    /// added to.</param>
    /// <param name="OutputPath">The output location for the class files. This
    /// is a fully qualified path.</param>
    public static void CreateAllTables(BulletedList StatusBulletList, string OutputPath)
    {
        DataTable table = ListDatabaseTables();

        if (table == null)
        {
            ListItem liRowName = new ListItem();
            liRowName.Text = "Database Error";
            StatusBulletList.Items.Add(liRowName);
            return;
        }

        // Loop through the tables and create the accessor classes
        foreach (DataRow row in table.Rows)
        {
            if (row["TABLE_NAME"].ToString() != "syssegments" && row["TABLE_NAME"].ToString() != "sysconstraints")
            {
                ListItem liRowName = new ListItem();
                liRowName.Text = row["TABLE_NAME"].ToString();
                StatusBulletList.Items.Add(liRowName);
                CreateSingleTable(StatusBulletList, OutputPath, row["TABLE_NAME"].ToString());
            }
        }
    }

    /// <summary>
    /// Returns a DataTable containing all the table names in the default
    /// database.
    /// </summary>
    /// <returns>DataTable listing the table names.</returns>
    public static DataTable ListDatabaseTables()
    {
        string SQL;
        DataTable table = null;

        // Grab all the table names from the current database
        SQL = "SELECT TABLE_NAME FROM information_schema.tables WHERE NOT TABLE_NAME='sysdiagrams' AND TABLE_SCHEMA = 'dbo' AND TABLE_TYPE= 'BASE TABLE'";
        table = DatabaseAccess.ExecSQL(SQL);

        return table;
    }

    /// <summary>
    /// Output a class file for the requested table in the current default database
    /// </summary>
    /// <param name="OutputPath">The output location for the class files. This
    /// is a fully qualified path.</param>
    /// <param name="TableName">The table name</param>
    public static void CreateSingleTable(string OutputPath, String TableName)
    {
        BulletedList bl = new BulletedList();
        CreateSingleTable(bl, OutputPath, TableName);
    }
    /// <summary>
    /// Output a class file for the requested table in the current default database
    /// </summary>
    /// <param name="OutputPath">The output location for the class files. This
    /// is a fully qualified path.</param>
    /// <param name="StatusBulletList">A BulletedList where status information can be
    /// added to.</param>
    /// <param name="TableName">The table name</param>
    public static void CreateSingleTable(BulletedList StatusBulletList, string OutputPath, String TableName)
    {
        string SQL;
        IDataReader reader = null;
        DataTable schema = null;
        List<TableFieldInfo> fields = new List<TableFieldInfo>();

        // Grab the current table
        SQL = "SELECT TOP 1 * FROM " + TableName;
        reader = ExecSQLReader("", SQL);

        // Get the table schema
        if (reader != null)
            schema = reader.GetSchemaTable();

        // Grab the field information we need
        if (schema != null)
        {
            foreach (DataRow myField in schema.Rows)
            {
                TableFieldInfo f = new TableFieldInfo();
                f.name = myField["ColumnName"].ToString();
                f.type = myField["ProviderSpecificDataType"].ToString();
                f.allowNull = bool.Parse(myField["AllowDBNull"].ToString());
                f.readOnly = bool.Parse(myField["IsReadOnly"].ToString());
                f.maxSize = int.Parse(myField["ColumnSize"].ToString());
                fields.Add(f);

                string info = "---> " + f.name + " (" + f.type + ")";
                if (f.readOnly)
                    info += " (RO)";
                else
                    info += " (RW)";
                if (f.allowNull)
                    info += " (Null)";
                StatusBulletList.Items.Add(new ListItem(info));
            }
        }
        else
        {
            ListItem liRowName = new ListItem();
            liRowName.Text = "Schema Error";
            StatusBulletList.Items.Add(liRowName);
        }

        // Clean the table name for the filesystem and c# names
        TableName = TableName.Replace('(', '_').Replace(')', '_').Replace('~', '_'); ;

        // Open the file
        string filename = OutputPath + "\\DBT_" + TableName + ".cs";
        StreamWriter sw = new StreamWriter(filename);

        // Add File Comments
        sw.WriteLine("// ");
        sw.WriteLine("// This file is auto generated based on the database");
        sw.WriteLine("// DO NOT MODIFY THIS FILE, EVER");
        sw.WriteLine("// Inherit this class and make changes there");
        sw.WriteLine("// DO NOT MODIFY THIS FILE, CHANGES WILL BE LOST");
        sw.WriteLine("// ");
        sw.WriteLine("");

        // Add Using statements
        sw.WriteLine("using System;");
        sw.WriteLine("using System.Collections.Generic;");
        sw.WriteLine("using System.Data;");
        sw.WriteLine("using System.Data.SqlClient;");
        sw.WriteLine("");

        // Open the class
        sw.WriteLine("public class DBT" + TableName + " : DatabaseAccess");
        sw.WriteLine("{");

        // Add accessors
        foreach (TableFieldInfo f in fields)
        {
            if (getType(f.type) == "int")
                sw.WriteLine("\tprivate " + getType(f.type) + " _" + f.name + " = -1;");
            else if (getType(f.type) == "float")
                sw.WriteLine("\tprivate " + getType(f.type) + " _" + f.name + " = -1;");
            else if (getType(f.type) == "DateTime")
                sw.WriteLine("\tprivate " + getType(f.type) + " _" + f.name + " = new DateTime(1753, 1, 1);");
            else if (getType(f.type) == "byte[]")
                sw.WriteLine("\tprivate " + getType(f.type) + " _" + f.name + " = new byte[1];");
            else
                sw.WriteLine("\tprivate " + getType(f.type) + " _" + f.name + ";");

            sw.WriteLine("\tpublic " + getType(f.type) + " " + f.name);
            sw.WriteLine("\t{");
            sw.WriteLine("\t\tget { return _" + f.name + "; }");
            string protect = "";
            if (f.readOnly)
                protect = "protected ";

            if (f.maxSize == 0 || getType(f.type) != "string")
                sw.WriteLine("\t\t" + protect + "set { _" + f.name + " = value; }");
            else
            {
                sw.WriteLine("\t\t" + protect + "set");
                sw.WriteLine("\t\t{");
                sw.WriteLine("\t\t\tif(value.Length <= " + f.maxSize.ToString() + ")");
                sw.WriteLine("\t\t\t\t_" + f.name + " = value;");
                sw.WriteLine("\t\t\telse");
                sw.WriteLine("\t\t\t\t_" + f.name + " = value.Substring(0, " + (f.maxSize).ToString() + ");");
                sw.WriteLine("\t\t}");
            }
            sw.WriteLine("\t}");
            sw.WriteLine("");
        }

        // Add the Constructors
        sw.WriteLine("\tprivate string _connectionString = \"\";");
        sw.WriteLine("\tpublic DBT" + TableName + "()");
        sw.WriteLine("\t{");
        sw.WriteLine("\t}");
        sw.WriteLine("");
        sw.WriteLine("\tpublic DBT" + TableName + "(string ConnectionString)");
        sw.WriteLine("\t{");
        sw.WriteLine("\t\t_connectionString = ConnectionString;");
        sw.WriteLine("\t}");
        sw.WriteLine("");
        sw.WriteLine("\tpublic DBT" + TableName + "(int itemID)");
        sw.WriteLine("\t{");
        sw.WriteLine("\t\tthis.Select(itemID);");
        sw.WriteLine("\t}");
        sw.WriteLine("");
        sw.WriteLine("\tpublic DBT" + TableName + "(string ConnectionString, int itemID)");
        sw.WriteLine("\t{");
        sw.WriteLine("\t\t_connectionString = ConnectionString;");
        sw.WriteLine("\t\tthis.Select(itemID);");
        sw.WriteLine("\t}");
        sw.WriteLine("");

        // Add the insert method
        StatusBulletList.Items.Add(new ListItem("<--- public void Insert()"));
        sw.WriteLine("\tpublic void Insert()");
        sw.WriteLine("\t{");
        foreach (TableFieldInfo f in fields)
        {
            if (f.readOnly == true)
            {
                sw.WriteLine("\t\tif(_" + f.name + " != -1)");
                sw.WriteLine("\t\t\treturn;");
                sw.WriteLine("");
            }
        }
        sw.Write("\t\tstring SQL = \"INSERT INTO " + TableName + " (");
        int count = fields.Count;
        foreach (TableFieldInfo f in fields)
        {
            count--;
            if (f.readOnly != true)
            {
                string fieldName = f.name;
                if (fieldName.ToUpper() == "DEFAULT")
                    fieldName = "[" + fieldName + "]";

                if (count != 0)
                    sw.Write(fieldName + ", ");
                else
                    sw.Write(fieldName);
            }
        }
        sw.Write(") VALUES (");
        count = fields.Count;
        foreach (TableFieldInfo f in fields)
        {
            count--;
            if (f.readOnly != true)
            {
                if (count != 0)
                    sw.Write("@" + f.name + ", ");
                else
                    sw.Write("@" + f.name);
            }
        }
        sw.WriteLine("); SELECT SCOPE_IDENTITY() AS ID;\";");
        sw.WriteLine("\t\tList<SqlParameter> parms = new List<SqlParameter>();");
        sw.WriteLine("\t\tDataTable table = null;");
        sw.WriteLine("");
        foreach (TableFieldInfo f in fields)
        {
            if (f.readOnly != true)
            {
                if (getType(f.type) == "DateTime")
                {
                    sw.WriteLine("\t\tif(_" + f.name + " != new DateTime(1753, 1, 1))");
                    sw.WriteLine("\t\t\tparms.Add(new SqlParameter(\"@" + f.name + "\", _" + f.name + "));");
                    sw.WriteLine("\t\telse");
                    sw.WriteLine("\t\t\tparms.Add(new SqlParameter(\"@" + f.name + "\", null));");
                }
                else
                {
                    sw.WriteLine("\t\tparms.Add(new SqlParameter(\"@" + f.name + "\", _" + f.name + "));");
                }
            }
        }
        sw.WriteLine("");
        sw.WriteLine("\t\tif (_connectionString == \"\")");
        sw.WriteLine("\t\t{");
        sw.WriteLine("\t\t\ttable = ExecSQL(SQL, parms);");
        sw.WriteLine("\t\t}");
        sw.WriteLine("\t\telse");
        sw.WriteLine("\t\t{");
        sw.WriteLine("\t\t\ttable = ExecSQL(_connectionString, SQL, parms);");
        sw.WriteLine("\t\t}");
        sw.WriteLine("\t\tif (table != null && table.Rows.Count == 1)");
        sw.WriteLine("\t\t{");
        foreach (TableFieldInfo f in fields)
        {
            if (f.readOnly == true)
                sw.WriteLine("\t\t\t_" + f.name + " = int.Parse(table.Rows[0][\"ID\"].ToString());");
        }
        sw.WriteLine("\t\t}");
        sw.WriteLine("\t}");
        sw.WriteLine("");


        // Add the update method
        StatusBulletList.Items.Add(new ListItem("<--- public void Update()"));
        sw.WriteLine("\tpublic void Update()");
        sw.WriteLine("\t{");
        sw.Write("\t\tstring SQL = \"UPDATE " + TableName + " SET ");
        count = fields.Count;
        foreach (TableFieldInfo f in fields)
        {
            count--;
            if (f.readOnly != true)
            {
                string fieldName = f.name;
                if (fieldName.ToUpper() == "DEFAULT")
                    fieldName = "[" + fieldName + "]";

                if (count != 0)
                    sw.Write(fieldName + "=@" + f.name + ", ");
                else
                    sw.Write(fieldName + "=@" + f.name + " ");
            }
        }
        sw.Write("WHERE ");
        foreach (TableFieldInfo f in fields)
        {
            string fieldName = f.name;
            if (fieldName.ToUpper() == "DEFAULT")
                fieldName = "[" + fieldName + "]";

            if (f.readOnly == true)
                sw.Write(fieldName + "=@" + f.name);
        }
        sw.WriteLine(";\";");
        sw.WriteLine("\t\tList<SqlParameter> parms = new List<SqlParameter>();");
        sw.WriteLine("");
        foreach (TableFieldInfo f in fields)
        {
            if (getType(f.type) == "DateTime")
            {
                sw.WriteLine("\t\tif(_" + f.name + " != new DateTime(1753, 1, 1))");
                sw.WriteLine("\t\t\tparms.Add(new SqlParameter(\"@" + f.name + "\", _" + f.name + "));");
                sw.WriteLine("\t\telse");
                sw.WriteLine("\t\t\tparms.Add(new SqlParameter(\"@" + f.name + "\", null));");
            }
            else
            {
                sw.WriteLine("\t\tparms.Add(new SqlParameter(\"@" + f.name + "\", _" + f.name + "));");
            }
        }
        sw.WriteLine("");
        sw.WriteLine("\t\tif (_connectionString == \"\")");
        sw.WriteLine("\t\t{");
        sw.WriteLine("\t\t\tExecSQL(SQL, parms);");
        sw.WriteLine("\t\t}");
        sw.WriteLine("\t\telse");
        sw.WriteLine("\t\t{");
        sw.WriteLine("\t\t\tExecSQL(_connectionString, SQL, parms);");
        sw.WriteLine("\t\t}");
        sw.WriteLine("\t}");
        sw.WriteLine("");

        // Add the select method
        bool CanSelect = false;
        foreach (TableFieldInfo f in fields)
        {
            if (f.readOnly == true && CanSelect == false)
            {
                StatusBulletList.Items.Add(new ListItem("<--- public bool Select(" + getType(f.type) + " " + f.name + ")"));
                sw.WriteLine("\tpublic bool Select(" + getType(f.type) + " " + f.name + ")");
                CanSelect = true;
            }
        }
        if (CanSelect == true)
        {
            sw.WriteLine("\t{");
            foreach (TableFieldInfo f in fields)
            {
                if (f.readOnly == true)
                    sw.WriteLine("\t\tstring SQL = \"SELECT * FROM " + TableName + " WHERE " + f.name + "=@" + f.name + "\";");
            }
            sw.WriteLine("\t\tList<SqlParameter> parms = new List<SqlParameter>();");
            sw.WriteLine("");
            foreach (TableFieldInfo f in fields)
            {
                if (f.readOnly == true)
                {
                    sw.WriteLine("\t\tSqlParameter parm" + f.name + " = new SqlParameter(\"@" + f.name + "\", " + f.name + ");");
                    sw.WriteLine("\t\tparms.Add(parm" + f.name + ");");
                }
            }
            sw.WriteLine("");
            sw.WriteLine("\t\tDataTable table = null;");
            sw.WriteLine("\t\tif (_connectionString == \"\")");
            sw.WriteLine("\t\t{");
            sw.WriteLine("\t\t\ttable = ExecSQL(SQL, parms);");
            sw.WriteLine("\t\t}");
            sw.WriteLine("\t\telse");
            sw.WriteLine("\t\t{");
            sw.WriteLine("\t\t\ttable = ExecSQL(_connectionString, SQL, parms);");
            sw.WriteLine("\t\t}");
            sw.WriteLine("\t\tif (table != null && table.Rows.Count == 1)");
            sw.WriteLine("\t\t{");
            sw.WriteLine("\t\t\tDataRow row = table.Rows[0];");
            sw.WriteLine("");
            foreach (TableFieldInfo f in fields)
            {
                if (getType(f.type) == "string")
                    sw.WriteLine("\t\t\t_" + f.name + " = row[\"" + f.name + "\"].ToString();");
                else if (getType(f.type) == "int")
                {
                    //sw.WriteLine("\t\t\t_" + f.name + " = int.Parse(row[\"" + f.name + "\"].ToString());");
                    sw.WriteLine("\t\t\tif(!int.TryParse(row[\"" + f.name + "\"].ToString(), out _" + f.name + "))");
                    sw.WriteLine("\t\t\t\t_" + f.name + " = -1;");
                }
                else if (getType(f.type) == "float")
                {
                    //sw.WriteLine("\t\t\t_" + f.name + " = int.Parse(row[\"" + f.name + "\"].ToString());");
                    sw.WriteLine("\t\t\tif(!float.TryParse(row[\"" + f.name + "\"].ToString(), out _" + f.name + "))");
                    sw.WriteLine("\t\t\t\t_" + f.name + " = -1;");
                }
                else if (getType(f.type) == "bool")
                {
                    //sw.WriteLine("\t\t\t_" + f.name + " = int.Parse(row[\"" + f.name + "\"].ToString());");
                    sw.WriteLine("\t\t\tif(!bool.TryParse(row[\"" + f.name + "\"].ToString(), out _" + f.name + "))");
                    sw.WriteLine("\t\t\t\t_" + f.name + " = false;");
                }
                else if (getType(f.type) == "DateTime")
                {
                    sw.WriteLine("\t\t\tif(row[\"" + f.name + "\"].ToString() != \"\")");
                    sw.WriteLine("\t\t\t\tDateTime.TryParse(row[\"" + f.name + "\"].ToString(), out _" + f.name + ");");
                    sw.WriteLine("\t\t\telse");
                    sw.WriteLine("\t\t\t\t_" + f.name + " = new DateTime(1753, 1, 1);");
                }
                else if (getType(f.type) == "byte[]")
                {
                    sw.WriteLine("\t\t\tif (row[\"Img\"].ToString() != \"\")");
                    sw.WriteLine("\t\t\t\t_" + f.name + " = (byte[])row[\"" + f.name + "\"];");
                }
                else
                    sw.WriteLine("\t\t\t//_" + f.name + " = row[\"" + f.name + "\"];");
            }
            sw.WriteLine("\t\t\treturn true;");
            sw.WriteLine("\t\t}");
            sw.WriteLine("\t\telse");
            sw.WriteLine("\t\t{");
            sw.WriteLine("\t\t\treturn false;");
            sw.WriteLine("\t\t}");
            sw.WriteLine("\t}");
            sw.WriteLine("");
        }

        // Add the SelectWhere method
        StatusBulletList.Items.Add(new ListItem("<--- public static DataTable SelectWhere(string WhereClause, List<SqlParameter> parms)"));
        sw.WriteLine("\tpublic static DataTable SelectWhere(string WhereClause, List<SqlParameter> parms)");
        sw.WriteLine("\t{");
        sw.WriteLine("\t\treturn SelectWhere(WhereClause, parms, \"\");");
        sw.WriteLine("\t}");
        sw.WriteLine("\tpublic static DataTable SelectWhere(string WhereClause, List<SqlParameter> parms, string connectionString)");
        sw.WriteLine("\t{");
        sw.WriteLine("\t\tstring SQL = \"SELECT * FROM " + TableName + " WHERE \" + WhereClause;");
        sw.WriteLine("");
        sw.WriteLine("\t\tDataTable table = null;");
        sw.WriteLine("\t\tif (connectionString == \"\")");
        sw.WriteLine("\t\t{");
        sw.WriteLine("\t\t\ttable = ExecSQL(SQL, parms);");
        sw.WriteLine("\t\t}");
        sw.WriteLine("\t\telse");
        sw.WriteLine("\t\t{");
        sw.WriteLine("\t\t\ttable = ExecSQL(connectionString, SQL, parms);");
        sw.WriteLine("\t\t}");
        sw.WriteLine("\t\treturn table;");
        sw.WriteLine("\t}");
        sw.WriteLine("");


        // Add the delete method
        if (CanSelect == true)
        {
            StatusBulletList.Items.Add(new ListItem("<--- public void Delete()"));
            sw.WriteLine("\tpublic void Delete()");
            sw.WriteLine("\t{");
            foreach (TableFieldInfo f in fields)
            {
                if (f.readOnly == true)
                {
                    sw.WriteLine("\t\tif(_" + f.name + " == -1)");
                    sw.WriteLine("\t\t\treturn;");
                    sw.WriteLine("");
                }
            }
            foreach (TableFieldInfo f in fields)
            {
                if (f.readOnly == true)
                    sw.WriteLine("\t\tstring SQL = \"DELETE FROM " + TableName + " WHERE " + f.name + "=@" + f.name + "\";");
            }
            sw.WriteLine("\t\tList<SqlParameter> parms = new List<SqlParameter>();");
            sw.WriteLine("");
            foreach (TableFieldInfo f in fields)
            {
                if (f.readOnly == true)
                {
                    sw.WriteLine("\t\tparms.Add(new SqlParameter(\"@" + f.name + "\", " + f.name + "));");
                }
            }
            sw.WriteLine("");
            sw.WriteLine("\t\tif (_connectionString == \"\")");
            sw.WriteLine("\t\t{");
            sw.WriteLine("\t\t\tExecSQL(SQL, parms);");
            sw.WriteLine("\t\t}");
            sw.WriteLine("\t\telse");
            sw.WriteLine("\t\t{");
            sw.WriteLine("\t\t\tExecSQL(_connectionString, SQL, parms);");
            sw.WriteLine("\t\t}");
            sw.WriteLine("\t}");
            sw.WriteLine("");
        }

        // Add the Table method
        StatusBulletList.Items.Add(new ListItem("<--- public static DataTable Table()"));
        sw.WriteLine("\tpublic static DataTable Table()");
        sw.WriteLine("\t{");
        sw.WriteLine("\t\treturn Table(\"\");");
        sw.WriteLine("\t}");
        sw.WriteLine("\tpublic static DataTable Table(string connectionString)");
        sw.WriteLine("\t{");
        sw.WriteLine("\t\tstring SQL = \"SELECT * FROM " + TableName + ";\";");
        sw.WriteLine("\t\tList<SqlParameter> parms = new List<SqlParameter>();");
        sw.WriteLine("");
        sw.WriteLine("\t\tif (connectionString == \"\")");
        sw.WriteLine("\t\t{");
        sw.WriteLine("\t\t\treturn ExecSQL(SQL, parms);");
        sw.WriteLine("\t\t}");
        sw.WriteLine("\t\telse");
        sw.WriteLine("\t\t{");
        sw.WriteLine("\t\t\treturn ExecSQL(connectionString, SQL, parms);");
        sw.WriteLine("\t\t}");
        sw.WriteLine("\t}");

        // Close the class
        sw.WriteLine("}");

        // Close the file
        sw.Close();
    }

    public static IDataReader ExecSQLReader(string DatabaseName, string SQL)
    {
        // Database access variables
        Database database = null;
        DbCommand command = null;
        IDataReader reader = null;

        try
        {
            if (DatabaseName != "")
                database = DatabaseFactory.CreateDatabase(DatabaseName);
            else
                database = DatabaseFactory.CreateDatabase();
            command = database.GetSqlStringCommand(SQL);
            reader = database.ExecuteReader(command);
        }
        catch (Exception e)
        {
            // Remove stupid warning
            Exception x = e;
            e = x;
        }

        return reader;
    }

    private static string getType(string DBType)
    {
        string ret = DBType;

        if (DBType == "System.Data.SqlTypes.SqlString")
            ret = "string";
        else if (DBType == "System.Data.SqlTypes.SqlInt16")
            ret = "Int16";
        else if (DBType == "System.Data.SqlTypes.SqlInt32")
            ret = "int";
        else if (DBType == "System.Data.SqlTypes.SqlFloat")
            ret = "float";
        else if (DBType == "System.Data.SqlTypes.SqlDouble")
            ret = "float";
        else if (DBType == "System.Data.SqlTypes.SqlDecimal")
            ret = "float";
        else if (DBType == "System.Data.SqlTypes.SqlBoolean")
            ret = "bool";
        else if (DBType == "System.Data.SqlTypes.SqlDateTime")
            ret = "DateTime";
        else if (DBType == "System.Data.SqlTypes.SqlByte")
            ret = "byte";
        else if (DBType == "System.Data.SqlTypes.SqlBinary")
            ret = "byte[]";

        return ret;
    }

    private class TableFieldInfo
    {
        public string name;
        public string type;
        public bool allowNull;
        public bool readOnly;
        public int maxSize;
    }
}
share|improve this question
    
You probably want to be using T4 for this. msdn.microsoft.com/en-us/library/bb126445.aspx is a reasonable place to start. –  Jeff Paulsen May 25 '11 at 14:50

1 Answer 1

up vote 8 down vote accepted

1) As already mentioned T4 might be a better solution though I'm not sure whether it is available for earlier VS versions.

2) I would avoid using string OutputPath parameters, Streams are usually more handy.

3) Separate presentation and business logic code. Having BulletedList parameter in such a class seems to be completely wrong to me.

4) I think you should not even take this BulletedList (or anything similar) as parameter. This messages log is an output of your methods, not an input, so there is no point accepting it as parameter. Return it and remove method overloads which are not needed.

5) Naming convention - usual rules are lower camel case for parameters.

6) Define variables closer to their first assignment and do not assign them any value if this value will be overwritten anyway. This is mostly about CreateSingleTable method. ListDatabaseTables also has 5 lines of code instead of maximum 3 needed.

7) I would make ListDatabaseTables() method more strongly typed. Returning a DataTable gives me absolutely no idea how to use it.

8) I prefer writing such constructions: 

            if (f.readOnly)
                info += " (RO)";
            else
                info += " (RW)";

as: 

info += f.readOnly ? " (RO)" : " (RW)";

This IMO shows more clearly that you're going to append something to info anyway and this something depends on isReadonly value.

9) I would replace this: 

string info = "---> " + f.name + " (" + f.type + ")";
if (f.readOnly)
    info += " (RO)";
else
    info += " (RW)";
if (f.allowNull)
    info += " (Null)";

with this: 

string info = string.Format("---> {0} ({1}) ({2}){3}"
                                 , f.name
                                 , f.type
                                 , f.readOnly? "RO" : "RW"
                                 , f.allowNull ? " (NULL)" : string.Empty);

This shows more clearly which format will info variable have. Also this has only one assignment which is better than doing += on string several times.

10) sw.WriteLine("// Inherit this class and make changes there"); I would prefer extending existing class instead of inheritance. Or at least you should allow extending it. In order to allow this generated classes are usually defined as partial.

11) A lot of repeats here: 

        if (getType(f.type) == "int")
            sw.WriteLine("\tprivate " + getType(f.type) + " _" + f.name + " = -1;");
        else if (getType(f.type) == "float")
            sw.WriteLine("\tprivate " + getType(f.type) + " _" + f.name + " = -1;");
        else if (getType(f.type) == "DateTime")
            sw.WriteLine("\tprivate " + getType(f.type) + " _" + f.name + " = new DateTime(1753, 1, 1);");
        else if (getType(f.type) == "byte[]")
            sw.WriteLine("\tprivate " + getType(f.type) + " _" + f.name + " = new byte[1];");
        else
            sw.WriteLine("\tprivate " + getType(f.type) + " _" + f.name + ";");

It should be separated into at least two blocks:

a) write private + type
b) determine default value and write it (if any)

12) You have new DateTime(1753, 1, 1); repeated several times in your code. I would consider this string as magic string and I think it should be extracted into constant.

13) fieldName.ToUpper() == "DEFAULT". string.Equals(...) has a parameter to ignore case.

14) You have just written your own string.Join(...) here: 

    count = fields.Count;
    foreach (TableFieldInfo f in fields)
    {
        count--;
        if (f.readOnly != true)
        {
            if (count != 0)
                sw.Write("@" + f.name + ", ");
            else
                sw.Write("@" + f.name);
        }
    }

15) if (f.readOnly == true && CanSelect == false) I do not think this is the case when bool variable should be compared against true or false. I would prefer if (f.readOnly && !CanSelect)

16) Do not write god methods/classes and do not instantiate god objects. CreateSingleTable is definitely a god method - it has almost 500 lines of code !!! Raptors will come for you as soon as they will finish with goto writers. Break down this method into ~5-10 smaller methods.

17)

private static string getType(string DBType)
{
    string ret = DBType;

    if (DBType == "System.Data.SqlTypes.SqlString")
        ret = "string";
    else if (DBType == "System.Data.SqlTypes.SqlInt16")
        ret = "Int16";
    else if (DBType == "System.Data.SqlTypes.SqlInt32")
        ret = "int";
    else if (DBType == "System.Data.SqlTypes.SqlFloat")
        ret = "float";
    else if (DBType == "System.Data.SqlTypes.SqlDouble")
        ret = "float";
    else if (DBType == "System.Data.SqlTypes.SqlDecimal")
        ret = "float";
    else if (DBType == "System.Data.SqlTypes.SqlBoolean")
        ret = "bool";
    else if (DBType == "System.Data.SqlTypes.SqlDateTime")
        ret = "DateTime";
    else if (DBType == "System.Data.SqlTypes.SqlByte")
        ret = "byte";
    else if (DBType == "System.Data.SqlTypes.SqlBinary")
        ret = "byte[]";

    return ret;
}

Instead of such constructions I usually use collections which contain mappings between input condition and desired result. I would replace your 10 conditions with this: 

Dictionary<string, string> dbTypeToDotNetTypeMappings =
    new Dictionary<string, string>();

dbTypeToDotNetTypeMappings.Add("System.Data.SqlTypes.SqlString", "string");
// ... 
dbTypeToDotNetTypeMappings.Add("System.Data.SqlTypes.SqlBinary", "byte[]");

string mappedType;
if (dbTypeToDotNetTypeMappings.TryGetValue(dbType, out mappedType))
    return mappedType;

return dbType;

This shows more clearly the fact that you have a lot of similar input->output transformations. Also it allows moving this conditions out of the code, to the configuration file for example.

18) getType() should be more strongly typed. It has to be Type -> Type mapping instead of string -> string.

19) 

    catch (Exception e)
    {
        // Remove stupid warning
        Exception x = e;
        e = x;
    }

The best way to get rid of warning is logging the exception. Otherwise use #pragma: 

#pragma warning disable 168
        catch (Exception e)
#pragma warning restore 168
        {

        }
share|improve this answer
    
For point 19, you could do catch (Exception) if you do not need the variable e. –  Matthew Mar 20 at 4:10

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.