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:
- This was developed and is being used for a ASP.NET project, which is the reason for the
BulletedList
. - 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;
}
}
-
\$\begingroup\$ You probably want to be using T4 for this. msdn.microsoft.com/en-us/library/bb126445.aspx is a reasonable place to start. \$\endgroup\$Jeff Paulsen– Jeff Paulsen2011年05月25日 14:50:56 +00:00Commented May 25, 2011 at 14:50
1 Answer 1
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
{
}
-
1\$\begingroup\$ For point 19, you could do
catch (Exception)
if you do not need the variablee
. \$\endgroup\$Matthew– Matthew2014年03月20日 04:10:41 +00:00Commented Mar 20, 2014 at 4:10