あなたはいくつかの問題を提起し、あなたが示しているコードはさらにいくつかの問題を提起しています。あなたが考えたいかもしれないこと:
データベース中心のアクティビティを独自のクラスに制限するようにしてください。GlobalStatsオブジェクトの行のリストを返す方法については、データベースからそのデータを引き出して実際のオブジェクトにする処理を行う実際のクラスを除いて、誰も知る必要はありません。誰も。他の誰かが知っている場合、あなたのクラスは情報隠蔽を使用していません (私たちはオブジェクト指向言語を使用しているため、使用する必要があります)。
オブジェクトが を実装している場合はIDisposable、try {} finally {}ブロックでラップするか、usingステートメントでラップする必要があります (以下の 2 番目の例を参照)。
接続文字列は、実際にそれを必要とするクラスにのみアクセスできるようにする必要があります (懸念事項の分離の一部)。おそらく、その情報を持つ基本 DataAccess クラスを持っていますか?
public abstract class DataAccess
{
protected const string ConnectionString = "YourConnectionStringHere";
}
その後、リポジトリはこのクラスから継承でき、コードが不必要に結合される原因となるグローバルな静的定数はありません。
あなたが書いているものを私が書く方法は次のとおりです(注意してください、このコードは実際に使用することを意図したものではなく、説明のみを目的としています):
[WebMethod]
public List<GlobalStat> GetStats()
{
GlobalStatsRepository repository = new GlobalStatsRepository();
List<GlobalStat> stats = repository.GetStats();
return stats;
}
データ アクセス層
public class GlobalStatsRepository
{
public List<GlobalStat> GetStats()
{
string sql = @"SELECT * from GlobalStats"; //no, not a good practice
var stats = new List<GlobalStat>();
try
{
string ConString = Constants.connString;
conn = new SqlConnection(ConString);
cmd = new SqlCommand(sql, conn);
conn.Open();
dr = cmd.ExecuteReader();
while (dr.Read())
{
GlobalStat stat = new GlobalStat();
stat.Key = dr[0].ToString();
stat.Value = int.Parse(dr[1].ToString());
stats.Add(stat);
}
}
catch (SQLDataReaderException ex)
{
logger.Log(ex);
throw;
}
return stats;
}
}
パラメータ化されたクエリの例
public List<GlobalStat> GetStatsById(int id)
{
var stats = new List<GlobalStat>();
string sql = @"SELECT * from GlobalStats WHERE Id = @Id";
using (SqlConnection conn = new SqlConnection(ConString))
{
conn.Open();
using (SQLCommand command = new SqlCommand(sql, conn))
{
command.Parameters.Add(new SqlParameter("Id", id));
SqlDataReader reader = command.ExecuteReader();
while (reader.Read())
{
GlobalStat stat = new GlobalStat();
stat.Key = dr[0].ToString();
stat.Value = int.Parse(dr[1].ToString());
stats.Add(stat);
}
}
}
return stats;
}