I have a database with a number of stored procedures. These stored procedures provide the basic CRUD operations against the data. I'm trying to create a DAO layer with separate DAOs for each domain class (Album, Artist, Genre, Review, etc) which uses a common Base DAO.
Using the Album DAO as an example, this is what I have so far:
public class AlbumDao
{
//Members
BaseDao baseDao = new BaseDao();
//Public Methods
public Album GetAlbumById(int id)
{
Album album;
//Get Album
List<Parameter> parameters = new List<Parameter>();
parameters.Add(new Parameter("@Id", SqlDbType.Int, id));
DataTable dataTable = baseDao.ExecuteQuery("GetAlbumById", parameters);
album = AlbumMapper(dataTable.Rows[0]);
//Return
return album;
}
public List<Album> GetAllAlbums()
{
return GetAlbumList("GetAlbums");
}
public List<Album> GetAllFiveStarAlbums()
{
List<Parameter> parameters = new List<Parameter>();
parameters.Add(new Parameter("@Rating", SqlDbType.Int, 5));
return GetAlbumList("GetAlbumsByRating", parameters);
}
public void InsertAlbum(Album newAlbum)
{
List<Parameter> parameters = new List<Parameter>();
parameters.Add(new Parameter("@Title", SqlDbType.VarChar, newAlbum.Title));
parameters.Add(new Parameter("@Composer", SqlDbType.VarChar, newAlbum.Composer));
parameters.Add(new Parameter("@ReleaseYear", SqlDbType.Int, newAlbum.ReleaseYear));
parameters.Add(new Parameter("@Rating", SqlDbType.Int, newAlbum.Rating));
parameters.Add(new Parameter("@IsFranchise", SqlDbType.Bit, newAlbum.IsFranchise));
baseDao.ExecuteNonQuery("AddAlbum", parameters);
}
//List Method
private List<Album> GetAlbumList(string procedureName, List<Parameter> parameters = null)
{
List<Album> albumList = new List<Album>();
try
{
DataTable dataTable = baseDao.ExecuteQuery(procedureName, parameters);
foreach (DataRow row in dataTable.Rows)
{
albumList.Add(AlbumMapper(row));
}
}
catch (Exception e)
{
Console.WriteLine(e.Message);
}
return albumList;
}
//Mappers
private Album AlbumMapper(DataRow dr)
{
Album album = new Album();
if (dr.Table.Columns.Contains("AlbumId"))
{
album.Id = Int32.Parse(dr["AlbumId"].ToString());
}
if (dr.Table.Columns.Contains("Title"))
{
album.Title = dr["Title"].ToString();
}
if (dr.Table.Columns.Contains("Composer"))
{
album.Composer = dr["Composer"].ToString();
}
if (dr.Table.Columns.Contains("ReleaseYear"))
{
album.ReleaseYear = Int32.Parse(dr["ReleaseYear"].ToString());
}
if (dr.Table.Columns.Contains("Rating"))
{
album.Rating = Int32.Parse(dr["Rating"].ToString());
}
if (dr.Table.Columns.Contains("isFranchase"))
{
album.IsFranchise = Boolean.Parse(dr["isFranchase"].ToString());
}
return album;
}
}
And this is the base DAO:
public class BaseDao
{
string connectionString = "xxx";
SqlConnection connection;
SqlCommand command;
SqlDataAdapter adapter;
public DataTable ExecuteQuery(string procedureName, List<Parameter> parameters = null)
{
DataTable dataTable = new DataTable();
using (connection = new SqlConnection(connectionString))
{
//Create Command
command = new SqlCommand(procedureName, connection);
command.CommandType = CommandType.StoredProcedure;
//Add Parameters If Exist
if (parameters != null)
{
foreach (Parameter parameter in parameters)
{
command.Parameters.Add(parameter.Name, parameter.Type).Value = parameter.Value;
}
}
//Populate DataTable With Stored Procedure Results
try
{
adapter = new SqlDataAdapter(command);
adapter.Fill(dataTable);
}
catch (Exception e)
{
Console.WriteLine(e.Message);
}
}
//Return
return dataTable;
}
public void ExecuteNonQuery(string procedureName, List<Parameter> parameters = null)
{
using (connection = new SqlConnection(connectionString))
{
//Create Command
command = new SqlCommand(procedureName, connection);
command.CommandType = CommandType.StoredProcedure;
//Add Parameters If Exist
if (parameters != null)
{
foreach (Parameter parameter in parameters)
{
command.Parameters.Add(parameter.Name, parameter.Type).Value = parameter.Value;
}
}
//Execute
try
{
connection.Open();
command.ExecuteNonQuery();
}
catch (Exception e)
{
Console.WriteLine(e.Message);
}
}
}
}
Here is the Album class (just a simple class with constructors and properties):
public class Album
{
//CONSTRUCTORS
public Album() { }
public Album(int id, string title, string composer, int releaseYear, int rating, bool isFranchise)
{
this.Id = id;
this.Title = title;
this.Composer = composer;
this.ReleaseYear = releaseYear;
this.Rating = rating;
this.IsFranchise = isFranchise;
}
public Album(string title, string composer, int releaseYear, int rating, bool isFranchise)
{
this.Title = title;
this.Composer = composer;
this.ReleaseYear = releaseYear;
this.Rating = rating;
this.IsFranchise = isFranchise;
}
//PROPERTIES
public int Id { get; set; }
public string Title { get; set; }
public string Composer { get; set; }
public int ReleaseYear { get; set; }
public int Rating { get; set; }
public bool IsFranchise { get; set; }
}
So for methods that return a single Album object such as GetAlbumById
, I execute the query by passing in the SP name and its parameters to the base DAO, and then take the data from row 0.
For methods that return a list of Album objects such as GetAllFiveStarAlbums
, I also call ExecuteQuery
but from the GetAlbumList
method that iterates through the DataTable and builds a list of Album objects.
In all cases, I use a mapper method to convert the DataTable row to an Album object.
You can imagine the DAOs for Artist, Review etc structured the same way.
I'm unsure if this is good design so I'd appreciate some feedback and advice?
Another thought that occurred to me is that the Album DAO could just have a single List method which takes the name of the stored procedure and SP parameters as parameters, but that would require passing that information from the BLL and I don't think the BLL should know about the names of database stored procedures.
-
\$\begingroup\$ Please post the code for Album. \$\endgroup\$paparazzo– paparazzo2018年07月04日 15:20:08 +00:00Commented Jul 4, 2018 at 15:20
-
\$\begingroup\$ @paparazzo I've posted the Album class now. Thanks. \$\endgroup\$John Steed– John Steed2018年07月04日 15:29:13 +00:00Commented Jul 4, 2018 at 15:29
2 Answers 2
DataTable has a lot of overhead.
In the code you have a lot of overhead. Album AlbumMapper is called for every row and you parse for every column name.
I think it would be a lot clean if you based is on Album and return List
public List<Album> ExecuteQuery(string procedureName, List<Parameter> parameters = null)
{
Use Album
also for the query. Have nullable Properties and null means don't search on that.
public List<Album> SearchAlbum(Album album)
{
List<Album> searchAlbum = new List<Album>();
List<Parameter> parameters = new List<Parameter>();
string spName = "GetAlbums";
if (album.Rating != null)
{
parameters.Add(new Parameter("@Rating", SqlDbType.Int, 5));
spName = "GetAlbumsByRating";
else if (album.Rating != null)
{
parameters.Add(new Parameter("@Id", SqlDbType.Int, id));
spName = "GetAlbumsByRating";
}
else if (album.Tile != null)
{
parameters.Clear();
parameters.Add(new Parameter("@Title", SqlDbType.VarChar, newAlbum.Title));
parameters.Add(new Parameter("@Composer", SqlDbType.VarChar, newAlbum.Composer));
parameters.Add(new Parameter("@ReleaseYear", SqlDbType.Int, newAlbum.ReleaseYear));
parameters.Add(new Parameter("@Rating", SqlDbType.Int, newAlbum.Rating));
parameters.Add(new Parameter("@IsFranchise", SqlDbType.Bit, newAlbum.IsFranchise));
spName = "AddAlbum";
}
using (connection = new SqlConnection(connectionString))
{
connection.Open();
//Create Command
command = new SqlCommand(spName, connection);
command.CommandType = CommandType.StoredProcedure;
if (parameters != null)
{
foreach (Parameter parameter in parameters)
{
command.Parameters.Add(parameter.Name, parameter.Type).Value = parameter.Value;
}
}
//here would need another branch for an insert
using(SqlDataReader rdr = command.Execute())
{
while(rdr.Read())
{
Album album = new Album();
album.ID = rdr.GetInt(0);
album.Title = rdr.GetString(1);
...
searchAlbum.Add(album);
}
}
}
return searchAlbum;
}
Why not save yourself all those ADO.NET and Datatable to custom class headaches and use an ORM like Dapper? And then you simply have a bunch of Services, e.g. an AlbumService
which has a Get(int id)
method, etc. And you can then have an IAlbumService
so you can do IoC/DI etc.
-
\$\begingroup\$ Thanks for your reply but let's assume I'm constrained to using vanilla ADO.NET for the moment. I wish to know the best way to structure the DAO layer. \$\endgroup\$John Steed– John Steed2018年07月04日 14:27:47 +00:00Commented Jul 4, 2018 at 14:27