3

I've been coding for a while, but still consider myself a beginner. I use very simplistic ADO.NET classes with inbuilt SQL statements. I'd like to hear from the community about what I'm doing wrong and how I can improve, and what the suggested next steps are to take my coding into current standards.

I'm really interested in trying EF, although I can't seem to find a tutorial that fits in with the way I do my BLL and DAL classes, so would appreciate a pointer in the right direction.

Basically if I have a Gift, I would create a Gift class (BLL\Gift.cs):

using MyProject.DataAccessLayer; namespace MyProject.BusinessLogicLayer { public class Gift { public int GiftID { get; set; } public string GiftName { get; set; } public string Description { get; set; } public decimal Price { get; set; } public static Gift GetGiftByID(int GiftID) { GiftDAL dataAccessLayer = new GiftDAL(); return dataAccessLayer.GiftsSelectByID(GiftID); } public void DeleteGift(Gift myGift) { GiftDAL dataAccessLayer = new GiftDAL(); dataAccessLayer.DeleteGift(myGift); } public bool UpdateGift(Gift myGift) { GiftDAL dataAccessLayer = new GiftDAL(); return dataAccessLayer.UpdateGift(myGift); } public int InsertGift(string GiftName, string Description, decimal Price) { Gift myGift = new Gift(); myGift.GiftName = GiftName; myGift.Description = Description; myGift.Price = Price; GiftDAL dataAccessLayer = new GiftDAL(); return dataAccessLayer.InsertGift(myGift); } } }

I then have a DAL class which holds my connection string (DAL\sqlDAL.css):

namespace MyProject.DataAccessLayer { public class SqlDataAccessLayer { public readonly string _connectionString = string.Empty; public SqlDataAccessLayer() { _connectionString = WebConfigurationManager.ConnectionStrings["SQLConnectionString"].ConnectionString; if (string.IsNullOrEmpty(_connectionString)) { throw new Exception("No database connection String found"); } } } }

and then a DAL class (DAL\giftDAL.cs) where I've shown a couple of the methods (Update and Delete):

using MyProject.BusinessLogicLayer; namespace MyProject.DataAccessLayer { public class GiftDAL : SqlDataAccessLayer { public bool UpdateGift(Gift GifttoUpdate) { string UpdateString = ""; UpdateString += "UPDATE Gifts SET"; UpdateString += "GiftName = @GiftName"; UpdateString += ",Description = @Description "; UpdateString += ",Price = @Price "; UpdateString += " WHERE GiftID = @GiftID"; int RowsAffected = 0; try { using (SqlConnection con = new SqlConnection(_connectionString)) { using (SqlCommand cmd = new SqlCommand(UpdateString, con)) { cmd.Parameters.AddWithValue("@GiftName", GifttoUpdate.GiftName); cmd.Parameters.AddWithValue("@Description", GifttoUpdate.Description); cmd.Parameters.AddWithValue("@Price ", GifttoUpdate.Price); cmd.Parameters.AddWithValue("@GiftID", GifttoUpdate.GiftID); con.Open(); RowsAffected = cmd.ExecuteNonQuery(); } } } catch (Exception ex) { Utils.LogError(ex.Message, ex.InnerException == null ? "N/A" : ex.InnerException.Message, ex.StackTrace); } return (RowsAffected == 1); } public void DeleteGift(Gift GifttoDelete) { string DeleteString = ""; DeleteString += "DELETE FROM GIFTS WHERE GIFTID = @GiftID"; try { using (SqlConnection con = new SqlConnection(_connectionString)) { using (SqlCommand cmd = new SqlCommand(DeleteString, con)) { cmd.Parameters.AddWithValue("@GiftID", GifttoDelete.GiftID); con.Open(); cmd.ExecuteNonQuery(); } } } catch (Exception ex) { Utils.LogError(ex.Message, ex.InnerException == null ? "N/A" : ex.InnerException.Message, ex.StackTrace); } } } }

So looking at that, how would you recommend I improve the code (if I continue to use ADO.NET) and what would my next step be to learn EF - or is there a better alternative?

Cheers, Robbie

Improve this question
2
  • 1
    I believe you should move your business logic to SQL Server Stored Procedures.Think about this,as long as a change happens in one of your tables,you have to change the code,this means you should adjust your code again compile+release.Make a library to call those stored procedures. Commented Feb 29, 2012 at 6:39
  • 1
    @Myra: not all business logic can be expressed inside stored procedures. Also, the data layer is conceptually (IMO) the wrong place for business logic. It's much harder to test and it's not portable. E.g. you can't switch to a lightweight SQLite in memory database for testing. Using an object relational mapper between your domain model and the persistent storage minimizes the effort required to match changes in either of the two worlds. So my advice is: keep business logic inside your code and use the DB as data storage. SPs should only relate to data concerns, never to business logic. Commented Feb 29, 2012 at 7:57

3 Answers 3

Reset to default
2

One thing that's always important (to me) is how testable a class is. I see a lot of explicit object construction in your code. Your Gift BL class explicitly depends on GiftDAL, which makes it very difficult to test the Gift class. Try to reduce the coupling between classes by making an abstraction of GiftDAL (e.g. an interface) and provide that to Gift from the outside (Dependency Injection).

A great book about good software design principle is Clean Code by Robert C. Martin. He establishes the SOLID principles. Check it out!

Also, be aware that you are now including persistence within your business logic class (or domain model as it is also called). This can be done (Active Record), but often people choose for a different approach nowadays where they separate their domain model from any infrastructure. The broad idea is: the fact that the objects need somehow be stored is important, but not important for the business logic itself, so those to concerns should be separated where possible. Often an Object Relational Mapper, for .NET NHibernate or Entity Framework are two examples for OR mappers.

Improve this answer
Sign up to request clarification or add additional context in comments.

1 Comment

This is excellent feedback, thank you very much. I seem to construct my objects a a lot. Eg, in my Page_Load if I'm binding a gift to a series of textboxes I'll do something like: ' if Gift myGift = Gift.GetGiftByID(Convert.ToInt32(Session["GiftIDtoEdit"])); txtGiftName.Text = myGift.GiftName; etc
2

If you want to stick with ADD.NET then why don't you look at Data Application Block from Microsoft Enterprise Library (current version is 5.0 May 2011) - it will allow you write vendor (MS-SQL/Oracle etc) neutral code easily and most of the boiler-plate coding get wrapped.

This is probably a simplest/shortest tutorial that I could find to get you started. However, MSDN link has plenty of information and see key scenario sections to jump-start.

Another suggestion is to use TransactionScope for managing transactions (instead of working directly with DbTransaction object).

Said all that I will recommend using Entity Framework (or any similar OR mapper tool - e.g. check NHibernet) because then you don't have to write typical code for basic CRUD operations. As far as your Dilemma goes, here is basic code snippet to get you started - I am using EF 4.1 with Code-First approach, POCO entities and Fluent API:

Entity:

public class Gift { public int Id { get; set; } public string Name { get; set; } public string Description { get; set; } public decimal Price { get; set; } }

Data Access Layer:

public class MyDbContext : DbContext { public DbSet<Gift> Gifts { get; set; } public MyDbContext () : base("name=[ConnStringName]") {} protected override void OnModelCreating(DbModelBuilder modelBuilder) { // Fluent API to provide mapping - you may use attributes in entity class var giftConfig = modelBuilder.Entity<Gift>(); giftConfig.Property(p => p.Id).HasColumnName("GiftID"); giftConfig.Property(p => p.Name).HasColumnName("GiftName"); giftConfig.Property(p => p.Description).HasColumnName("Description"); giftConfig.Property(p => p.Price).HasColumnName("Price"); giftConfig.HasKey(p => p.Id); base.OnModelCreating(modelBuilder); } }

Business Layer:

public static class GiftManager { public static Gift GetById(int id) { using(var db = new MyDbContext()) { return db.Gifts.Find(id); } } public static void Add(Gift gift) { using(var db = new MyDbContext()) { // do validation ... db.Gifts.Add(gift); // do auditing ... db.SaveChanges(); } } public static void Update(Gift gift) { using(var db = new MyDbContext()) { // do validation ... var entity = db.Sessions.Find(gift.Id); entity.Name = gift.Name; entity.Description = gift.Description; entity.Price = gift.Price; // do auditing ... db.SaveChanges(); } } }
Improve this answer

2 Comments

Thanks, I've never heard of this :-o. I'll have a look around to get some more info, but isn't EF Microsoft's preferred data access technology these days?
@RobbieMills, MS is a bit late-comer on OR-Mapping tool - EF is still evolving product and has some way to go. So yes, it is in active development (but ADO.NET still remains the underlying data-access mechanism. You see more focus on EF because its relatively new not to mention, it gives a great productivity boost.
0

I have a dal class whose methods return only bindable objects like datatable, List, etc. Nothing more or less. Then all business logic happens naturally in code behind in an aspnet application. Making backing objects is just too much work in most cases and overkill. I am comfortable with datatables and lists.

Improve this answer

3 Comments

What do you mean by backing objects? Sorry, I don't understand the term. :-)
I'm referring to an aspnet application where you would be applying data to grids, drop down menus, etc.
Funny, I just read this again months later. by "backing objects" I meant object relational mapper. Basically it would be a class that represented your database table. For example, you'd transform the table "Car" into a c# class called "Car" and map all the columns from the table into class member variables. There are plenty of easy ways to do that though, using LINQ, entity framework, or any other OR-mapper.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.