2
\$\begingroup\$

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.

asked Jul 4, 2018 at 11:41
\$\endgroup\$
2
  • \$\begingroup\$ Please post the code for Album. \$\endgroup\$ Commented Jul 4, 2018 at 15:20
  • \$\begingroup\$ @paparazzo I've posted the Album class now. Thanks. \$\endgroup\$ Commented Jul 4, 2018 at 15:29

2 Answers 2

1
\$\begingroup\$

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;
}
answered Jul 4, 2018 at 15:52
\$\endgroup\$
0
\$\begingroup\$

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.

answered Jul 4, 2018 at 14:17
\$\endgroup\$
1
  • \$\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\$ Commented Jul 4, 2018 at 14:27

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.