views:

2560

answers:

5

I'm trying to figure out the cleanest way to do this.

Currently I have a customer object:

public class Customer
{
    public int Id {get;set;}
    public string name {get;set;}
    public List<Email> emailCollection {get;set}
    public Customer(int id)
    {
        this.emailCollection = getEmails(id);
    }
}

Then my Email object is also pretty basic.

public class Email
{
    private int index;
    public string emailAddress{get;set;}
    public int emailType{get;set;}
    public Email(...){...}
    public static List<Email> getEmails(int id)
    {
        return DataAccessLayer.getCustomerEmailsByID(id);
    }
}

The DataAccessLayer currently connects to the data base, and uses a SqlDataReader to iterate over the result set and creates new Email objects and adds them to a List which it returns when done.

So where and how can I improve upon this?

Should I have my DataAccessLayer instead return a DataTable and leave it up to the Email object to parse and return a List back to the Customer?

I guess "Factory" is probably the wrong word, but should I have another type of EmailFactory which takes a DataTable from the DataAccessLayer and returns a List to the Email object? I guess that kind of sounds redundant...

Is this even proper practice to have my Email.getEmails(id) as a static method?

I might just be throwing myself off by trying to find and apply the best "pattern" to what would normally be a simple task.

Thanks.


Follow up

I created a working example where my Domain/Business object extracts a customer record by id from an existing database. The xml mapping files in nhibernate are really neat. After I followed a tutorial to setup the sessions and repository factories, pulling database records was pretty straight forward.

However, I've noticed a huge performance hit.

My original method consisted of a Stored Procedure on the DB, which was called by a DAL object, which parsed the result set into my domain/business object.

I clocked my original method at taking 30ms to grab a single customer record. I then clocked the nhibernate method at taking 3000ms to grab the same record.

Am I missing something? Or is there just a lot of overhead using this nhibernate route?

Otherwise I like the cleanliness of the code:

protected void Page_Load(object sender, EventArgs e)
{
    ICustomerRepository repository = new CustomerRepository();
    Customer customer = repository.GetById(id);
}

public class CustomerRepository : ICustomerRepository
        {
            public Customer GetById(string Id)
            {
                using (ISession session = NHibernateHelper.OpenSession())
                {
                    Customer customer = session
                                        .CreateCriteria(typeof(Customer))
                                        .Add(Restrictions.Eq("ID", Id))
                                        .UniqueResult<Customer>();
                    return customer;
                }
            }
        }

The example I followed had me create a helper class to help manage the Session, maybe that's why i'm getting this overhead?

public class NHibernateHelper
    {
        private static ISessionFactory _sessionFactory;
        private static ISessionFactory SessionFactory
        {
            get
            {
                if (_sessionFactory == null)
                {
                    Configuration cfg = new Configuration();
                    cfg.Configure();
                    cfg.AddAssembly(typeof(Customer).Assembly);
                    _sessionFactory = cfg.BuildSessionFactory();
                }
                return _sessionFactory;
            }

        }

        public static ISession OpenSession()
        {
            return SessionFactory.OpenSession();
        }
    }

With the application i'm working on, speed is of the essence. And ultimately a lot of data will pass between the web-app and the database. If it takes an agent 1/3 of a second to pull up a customer record as opposed to 3 seconds, that would be a huge hit. But if i'm doing something weird and this is a one time initial setup cost, then it might be worth it if the performance was just as good as executing stored procedures on the DB.

Still open to suggestions!


Updated.

I'm scrapping my ORM/NHibernate route. I found the performance is just too slow to justify using it. Basic customer queries just take too long for our environment. 3 seconds compared to sub-second responses is too much.

If we wanted slow queries, we'd just keep our current implementation. The idea to rewrite it was to drastically increase times.

However, after having played with NHibernate this past week, it is a great tool! It just doesn't quite fit my needs for this project.

+4  A: 

If the configuration you've got works now, why mess with it? It doesn't sound like you're identifying any particular needs or issues with the code as it is.

I'm sure a bunch of OO types could huddle around and suggest various refactorings here so that the correct responsibilities and roles are being respected, and somebody might even try to shoehorn in a design pattern or two. But the code you have now is simple and sounds like it doesn't have any issues - i'd say leave it.

Barry Fandango
I consider myself an OO guy, but I fulheartedly agree. The current solution is so simple that even I can understand it, which is definitely a good quality!
Treb
+4  A: 

I've implemented a DAL layer by basically doing what NHibernate does but manually. What NHibernate does is create a Proxy class that inherits from your Domain object (which should have all its fields marked as virtual). All data access code goes into property overrides, its pretty elegant actually.

I simplified this somewhat by having my Repositories fill out the simple properties themselves and only using a proxy for Lazy loading. What I ended up is a set of classes like this:

public class Product {
  public int Id {get; set;}
  public int CustomerId { get; set;}
  public virtual Customer Customer { get; set;}
}
public class ProductLazyLoadProxy {
  ICustomerRepository _customerRepository;
  public ProductLazyLoadProxy(ICustomerRepository customerRepository) {
    _customerRepository = customerRepository;
  }
  public override Customer {
    get {
      if(base.Customer == null)
        Customer = _customerRepository.Get(CustomerId);
      return base.Customer
    }
    set { base.Customer = value; }
  }
}
public class ProductRepository : IProductRepository {
  public Product Get(int id) {
    var dr = GetDataReaderForId(id);
    return new ProductLazyLoadProxy() {
      Id = Convert.ToInt(dr["id"]),
      CustomerId = Convert.ToInt(dr["customer_id"]),
    }
  }
}

But after writing about 20 of these I just gave up and learned NHibernate, with Linq2NHibernate for querying and FluentNHibernate for configuration nowadays the roadblocks are lower than ever.

George Mauer
+2  A: 

This may be too radical for you and doesn't really solve the question, but how about completely scrapping your data layer and opting for an ORM? You will save a lot of code redundancy that spending a week or so on a DAL will bring.

That aside, the pattern you're using resembles a repository pattern, sort of. I'd say your options are

  • A service object in your Email class - say EmailService - instantiated in the constructor or a property. Accessed via an instance such as email.Service.GetById(id)
  • A static method on Email, like Email.GetById(id) which is a similar approach
  • A completely separate static class that is basically a façade class, EmailManager for example, with static methods like EmailManager.GetById(int)
  • The ActiveRecord pattern where you are dealing with an instance, like email.Save() and email.GetById()
Chris S
+2  A: 

Most likely your application has its domain logic setup in transaction scripts. For .NET implementations that use transaction script Martin Fowler recommends the usage of the table data gateway pattern. .NET provides good support for this pattern because the table data gateway pattern is great with record set, which Microsoft implements with its DataSet-type classes.

Various tools within the Visual Studio environment should increase your productivity. The fact that DataSets can easily be databound to various controls (like the DataGridView) makes it a good choice for data-driven applications.

If your business logic is more complex than a few validations a domain model becomes a good option. Do note that a domain model comes with a whole different set of data access requirements!

David Walschots
A: 

I've only looked at NHibernate briefly before, but i'll look at it again this weekend and try to get a few samples working. My initial thoughts where that NHibernate might be over kill, but that might just be due to my lack of understanding about it.

Thanks for all the suggestions.

Dave
Oh yes, I definitely recommend you to do that! Mind you, I know next to nothing about NHibernate myself, but the little bits that I do know have me looking for a project where I can use it.
Treb