views:

116

answers:

3

OK, I'm always trying to improve the way I code because it's my passion. I have a .dbml file (linq-to-sql) and I use it to access my MS SQL database.

Imagine, if you will, that you have a Person table in your database and you want to provide a way to delete,add,modify a Person record.

The way I'm handling things currently is creating classes called PersonRepository, CarRepository, DocumentRepository, etc. For each table in my database, I create a repository class.

These repository classes generally consist of something similar to this:

MyDatabaseContext db = new MyDatabaseContext();

public Person GetPersonByID(int id)
{
    return db.Person.Where(p => p.ID == id);
}

Pretty much the same for the basic CRUD functionality of each table.

If I need something more specific, for example "Sergio, I need a list of all people born between x and y"; then I just add the method to the PersonRepository class.

public List<Person> GetPeopleFromDOB(DateTime x, DateTime y)
{
   //Do the logic here.
}

Another idea I had was to create a DataAccess.cs class and have all of these methods in there (we would be talking around 4-5 methods per tables in existance) and have them divided by regions.

What are the more knowledgeable programmers doing and what suggestions would you offer for an eager young programmer (I'm 20 years old). Thanks so much for your time!

+3  A: 

Here are the problems with what you're doing above:

Using List when you should be using IQueryable

http://stackoverflow.com/questions/1106802/why-use-asqueryable-instead-of-list

Basically, you should never get data until you need it. With your approach you're always getting data from the database and subsequent LINQ queries will act on all data. Why not just build up queries and let LINQ refine things down to where it only gets what you need and only when you need it?

Using Methods when Extension Methods would be perfect

You are creating a utility class of methods with things like GetPeopleFromDOB. Why not make this an Extension method? If you make all of them return IQueryable you could use them in succession as well. E.g. GetPeople().StatusEnabled().BornInJuly().AgeIsGreaterThan( 57 ); Also, if you must do this at least consider doing so in a partial class or a static utility class.

Consider Using ActiveRecord/Repository Pattern as the root

http://compiledexperience.com/blog/posts/Implementing-an-ActiveRecord-pattern-in-Linq-to-SQL

Right now you're creating several hardcoded repositories, but you should be basing them off of Repository?

Why not use validators?

If you're using ASP.NET MVC, validation is part and parcel, however with webforms you can also use the Data Annotation Validators.

http://adventuresdotnet.blogspot.com/2009/08/aspnet-webforms-validation-with-data.html

Nissan Fan
(EDIT: This is for a Windows Form application) Thanks for those links. IQueryable filters things in the database, whereas List<> filters things in memory. I'll have to figure out which is best for me.Also, do you have a useful link on how to use Extension Methods in this context?
Sergio Tapia
I'd stick to IQueryable. If you really want/need a query to be performed in memory then just add .ToList() on the result of your repository method call then do your filtering.(EDIT: Here's a link for making "Fluent" L2S extension methods. http://blog.sb2.fr/post/2009/01/03/Using-IQueryable3cT3e-Extension-Methods-With-LinqToSql.aspx
whatispunk
Here you go, and notice they have IQueryable: http://weblogs.asp.net/timothykhouri/archive/2007/09/12/linq-to-sql-and-extension-methods.aspx I point this out because it's a good example of using Extension methods with LINQ to SQL and also highlights cavaets.
Nissan Fan
What benefits does using an ActiveRecord pattern bring me? I feel it adds complexity where it isn't needed. If I use this SingleRepository approach, added functionality is much easier and the code results in something more natural to invoke "PersonRepository.FindPeople(), .FindPerson(), FindWomen(), FindMen(), etc."
Sergio Tapia
It genericizes data access and creates consistency across your DAL/BLL. In addition, it abstracts out data access and allows you to switch the underlying data tier out to another technology such as Entity Framework or NHibernate with little pain.
Nissan Fan
+2  A: 

I'd stick with the Single Responsibility principal and stick with your repository classes. If your DB grows over time, imagine how big the DataAccess class could become. You'll get away with it for a while, but it will grow and grow to the point where you have thousands of lines of code in a class (I have seen 15000+ lines in such a class before) and you're stuck!

Colin Desmond
Oh God. 15,000? I feel sorry for the guy that has to maintain that.
Sergio Tapia
+2  A: 

You'll want to use a REPOSITORY to connect directly with your DBML using L2S.

Then You'll want to create a SERVICE that talks to your Repository

Finally you'll use the Service in things like your code behind or your MVC Controller.

User Repository

Namespace Data

#Region "Interface"
    Public Interface IUserRepository
        Function GetAllUsers() As IList(Of User)
        Function GetUserByID(ByVal id As Integer) As User
    End Interface
#End Region

#Region "Repository"
    Public Class UserRepository : Implements IUserRepository
        Private dc As MyDataContext
        Public Sub New()
            dc = New MyDataContext
        End Sub

        Public Function GetAllUsers() As System.Collections.Generic.IList(Of User) Implements IUserRepository.GetAllUsers
            Dim users = From u In dc.Users
                        Select u
            Return users.ToList
        End Function

        Public Function GetUserByID(ByVal id As Integer) As User Implements IUserRepository.GetUserByID
            Dim viewUser = (From u In dc.Users
                       Where u.ID = id
                       Select u).FirstOrDefault
            Return viewUser
        End Function

    End Class
#End Region
End Namespace

User Service

Namespace Data

#Region "Interface"
    Public Interface IUserService
        Function GetAllUsers() As IList(Of User)
        Function GetUserByID(ByVal id As Integer) As User
    End Interface
#End Region


#Region "Service"
    Public Class UserService : Implements IUserService

        Private _ValidationDictionary As IValidationDictionary
        Private _UserRepository As IUserRepository

        Public Sub New(ByVal validationDictionary As IValidationDictionary, ByVal UserRepository As IUserRepository)
            _ValidationDictionary = validationDictionary
            _UserRepository = UserRepository
        End Sub

        Public Function GetAllUsers() As System.Collections.Generic.IList(Of User) Implements IUserService.GetAllUsers
            Return _UserRepository.GetAllUsers
        End Function


        Public Function GetUserByID(ByVal id As Integer) As User Implements IUserService.GetUserByID
            Return _UserRepository.GetUserByID(id)
        End Function

    End Class
#End Region
End Namespace

Now just make sure to use CRUD like you mentioned above.

Also, please forgive my VB. You might need to run it through the http://converter.telerik.com Code Converter to get the C# stuff.

rockinthesixstring
I don't understand the purpose of your Service here. You just seem to be wrapping the repository methods without actually adding any value. What does the validation dictionary do? Do you have a link you could share that explains more about this pattern? thanks.
fearofawhackplanet