c# - 私のコードを強化するためのガイド

Question

このプログラムは、テーブル 1 内のすべてのレコードをテーブル 2 にコピーし、テキスト ファイルにも書き込みます。すべてのレコードのコピーが完了すると、新しいレコードが追加される前に、レコードが削除され、table1 が空になります。たとえば、コードを強化したい：

1. レコードが空かどうかを確認するコードを挿入したり、ファイルのコピーで問題が発生した場合、または EOF の場合はどうすればよいですか??
2. このコードは form_load() 内にあり、win フォーム アプリケーションで実行されていました。プログラム exe を実行すると、フォームが表示されない場合はどうなりますか? このプログラムを背後のウィンドウで実行されているようにしたい。エラーまたは成功のメッセージボックスのみが表示されますか?
3. 解決策、ガイダンス、またはリファレンスのヘルプは非常に感謝しています。

前もって感謝します！

//create connection
 SqlConnection sqlConnection1 =
   new SqlConnection("Data Source=.\SQLEXPRESS;Database=F:\Test2.mdf;Integrated Security=True;User Instance=True");
//command insert into queries
  SqlCommand cmdCopy = new SqlCommand();
  cmdCopy.CommandType = System.Data.CommandType.Text;
  cmdCopy.CommandText = "INSERT INTO tblSend (ip, msg, date) SELECT ip, msg, date FROM tblOutbox";
  cmdCopy.Connection = sqlConnection1;
//insert into text file
  SqlCommand cmd = new SqlCommand();
  cmd.CommandType = CommandType.Text;
  cmd.CommandText = "SELECT * FROM tblOutbox";
  cmd.Connection = sqlConnection1;
  sqlConnection1.Open();
  StreamWriter tw = File.AppendText("c:\INMS.txt");
  SqlDataReader reader = cmd.ExecuteReader();
  tw.WriteLine("id, ip address, message, datetime");
  while (reader.Read())
  {
   tw.Write(reader["id"].ToString());
   tw.Write(", " + reader["ip"].ToString());
   tw.Write(", " + reader["msg"].ToString());
   tw.WriteLine(", " + reader["date"].ToString());
  }
  tw.WriteLine("Report Generate at : " + DateTime.Now);
  tw.WriteLine("---------------------------------");
  tw.Close();
  reader.Close();
//command delete
  String strDel = "DELETE tblOutbox";
  SqlCommand cmdDel = new SqlCommand(strDel, sqlConnection1);
//sqlConnection1.Open(); //open con

  cmdCopy.ExecuteScalar();
  cmd.ExecuteNonQuery(); //execute insert query
  cmdDel.ExecuteScalar();//execute delete query

  sqlConnection1.Close(); //close con
//*****************************************************

}
catch (System.Exception excep)
{
  MessageBox.Show(excep.Message);
}

Answer 1

いくつかの提案:

• フォームの外に移動します。ビジネス ロジックとデータ アクセスは、フォーム (ビュー) には属しません。別のクラスに移動します。
• MessageBox コードをフォームに保持します。それが表示ロジックです。try..catch 全体をメソッドの外に移動できます。メソッドに例外をスローさせるだけです。System.Exception をキャッチしないでください - 期待するデータベースのものをキャッチしてください。
• IDisposable と using ステートメントに関する Ty のコメントに同意します。
• Extract MethodとSingle Responsibility Principleを読んでください。このメソッドは多くのことを行い、長いです。それを壊します。
• 文字列ハードコードの一部を移動します。接続文字列またはファイル パスが変更された場合はどうなりますか? それらを構成ファイルに入れてみませんか (または、少なくともいくつかの定数を使用しませんか)。

とりあえず、まずは。:)

Answer 2

それは確かにいくつかのコードであり、気になる場合は、それを改善するために多くのことをお勧めできます。

最初にIDisposableを読み、その DataReader を次のように書き直します。

using(StreamWriter tw = File.AppendText("c:\INMS.txt"))
{
    using(SqlDataReader reader = cmd.ExecuteReader())
    {
      tw.WriteLine("id, ip_add, message, datetime");
      while (reader.Read())
      {
         tw.Write(reader["id"].ToString());
         tw.Write(", " + reader["ip_add"].ToString());
         tw.Write(", " + reader["message"].ToString());
         tw.WriteLine(", " + reader["datetime"].ToString());
      }
      tw.WriteLine(DateTime.Now);
      tw.WriteLine("---------------------------------");
   }
}

次に、キャッチの後に次のコードを追加し、クローズ コールを削除します。

finally
{
   sqlConnection1.Dispose(); //close con
}

Answer 3

まず、スキルの向上に努め、このようなコードを公開することにオープンであることを称賛します。より良いプログラマーになるための最初のステップは、このような態度を持つことだと私は信じています。

これは、いくつかの質問に答える実装です。古いコードの一部をメソッドに抽出し、責任の一部を独自のクラスに移動しました。

免責事項:

• コードはコンパイルされますが、データベースに対しては実行していません。
• 正確な要件がわからない特定のリファクタリングを手短に終わらせ、いくつかの概念を単純に保つようにしなければなりませんでした。

.

using System;
using System.Configuration;
using System.Data.SqlClient;
using System.IO;

// Program.cs
static class Program
{
    [STAThread]
    static void Main()
    {
        try
        {
            MailArchiver.Run();
            Console.WriteLine("Application completed successfully");
        }
        catch (Exception ex)
        {
            Console.WriteLine("Unexpected error occurred:");
            Console.WriteLine(ex.ToString());
        }

    }
}

// Reads new messages from DB, save it to a report file 
// and then clears the table
public static class MailArchiver
{

    public static void Run()
    {
        // Might be a good idea to a datetime suffix 
        ReportWriter.WriteFile(@"c:\INMS.txt");
        CopyAndClearMessages();
    }

    private static void CopyAndClearMessages()
    {
        SqlConnection cn = DbConnectionFactory.CreateConnection();
        cn.Open();

        try
        {
            SqlTransaction tx = cn.BeginTransaction();

            try
            {
                CopyMessages(cn, tx);
                DeleteMessages(cn, tx);
                tx.Commit();
            }
            catch
            {
                tx.Rollback();
                throw;
            }
        }
        finally
        {
            cn.Close();
        }
    }

    private static void DeleteMessages(SqlConnection cn, SqlTransaction tx)
    {
        var sql = "DELETE FROM tblOutbox";
        var cmd = new SqlCommand(sql, cn, tx);
        cmd.CommandTimeout = 60 * 2;  // timeout 2 minutes 
        cmd.ExecuteNonQuery();
    }

    private static void CopyMessages(SqlConnection cn, SqlTransaction tx)
    {
        var sql = "INSERT INTO tblSend (ip, msg, date) SELECT ip, msg, date FROM tblOutbox";
        var cmd = new SqlCommand(sql, cn, tx);
        cmd.CommandTimeout = 60 * 2;  // timeout 2 minutes 
        cmd.ExecuteNonQuery();
    }
}

// Provides database connections to the rest of the app.
public static class DbConnectionFactory
{
    public static SqlConnection CreateConnection()
    {
        // Retrieve connection string from app.config
        string connectionString = ConfigurationManager.ConnectionStrings["MailDatabase"].ConnectionString;
        var cn = new SqlConnection(connectionString);

        return cn;
    }
}

// Writes all the data in tblOutbox to a CSV file
public static class ReportWriter
{
    private static SqlDataReader GetData()
    {
        SqlConnection cn = DbConnectionFactory.CreateConnection();
        cn.Open();

        try
        {
            var cmd = new SqlCommand();
            cmd.CommandText = "SELECT * FROM tblOutbox";
            cmd.Connection = cn;

            return cmd.ExecuteReader();
        }
        finally
        {
            cn.Close();
        }
    }

    public static void WriteFile(string filename)
    {
        if (File.Exists(filename))
        {
            // This might be serious, we may overwrite data from the previous run.
            // 1. You might want to throw your own custom exception here, should want to handle this
            //    condition higher up.
            // 2. The logic added here is not the best and added for demonstration purposes only.
            throw new Exception(String.Format("The file [{0}] already exists, move the file and try again"));
        }
        var tw = new StreamWriter(filename);

        try
        {
            // Adds header record that describes the file contents
            tw.WriteLine("id,ip address,message,datetime");

            using (SqlDataReader reader = GetData())
            {
                while (reader.Read())
                {
                    var id = reader["id"].ToString();
                    var ip = reader["ip"].ToString();

                    //msg might contain commas, surround value with double quotes
                    var msg = reader["msg"].ToString();
                    var date = reader["data"].ToString();

                    if (IfValidRecord(id, ip, msg, msg, date))
                    {
                        tw.WriteLine(string.Format("{0},{1},{2},{3}", id, ip, msg, date));
                    }
                }

                tw.WriteLine("Report generated at : " + DateTime.Now);
                tw.WriteLine("--------------------------------------");
            }

        }
        finally
        {
            tw.Close();
        }

    }

    private static bool IfValidRecord(string id, string ip, string msg, string msg_4, string date)
    {
        // this answers your question on how to handle validation per record.
        // Add required logic here
        return true;
    }
}

Answer 4

すでに与えられた他の回答のいくつかに加えて、Transactionでデータ操作を保護することを検討することもできます。

次の操作を部分的に完了させたくない場合を想定しています。

  cmdCopy.ExecuteScalar();
  cmd.ExecuteNonQuery(); //execute insert query
  cmdDel.ExecuteScalar();//execute delete query

多くの行を処理している場合、更新をバッチ処理する必要があるかもしれませんが、それはまったく別の問題です。

Answer 5

1. SELECT クエリを使用して、空でない行を見つけます (EOF の問題に対処しているようです)。
2. イベントform_load時に、プログラムの引数によってフォームを非表示にします。
3. INSERT INTO (そして DELETE)を使用しないのはなぜですか?