Object oriented n-tier design. Am I abstracting too much? Or not enough?

Question

I'm building my first enterprise grade solution (at least I'm attempting to make it enterprise grade). I'm trying to follow best practice design patterns but am starting to worry that I might be going too far with abstraction.

I'm trying to build my asp.net webforms (in C#) app as an n-tier application. I've created a Data Access Layer using an XSD strongly-typed dataset that interfaces with a SQL server backend. I access the DAL through some Business Layer Objects that I've created on a 1:1 basis to the datatables in the dataset (eg, a UsersBLL class for the Users datatable in the dataset). I'm doing checks inside the BLL to make sure that data passed to DAL is following the business rules of the application. That's all well and good. Where I'm getting stuck though is the point at which I connect the BLL to the presentation layer. For example, my UsersBLL class deals mostly with whole datatables, as it's interfacing with the DAL. Should I now create a separate "User" (Singular) class that maps out the properties of a single user, rather than multiple users? This way I don't have to do any searching through datatables in the presentation layer, as I could use the properties created in the User class. Or would it be better to somehow try to handle this inside the UsersBLL?

Sorry if this sounds a little complicated... Below is the code from the UsersBLL:

using System;
using System.Data;
using PedChallenge.DAL.PedDataSetTableAdapters;

[System.ComponentModel.DataObject]
public class UsersBLL
{
    private UsersTableAdapter _UsersAdapter = null;
    protected UsersTableAdapter Adapter
    {
        get
        {
            if (_UsersAdapter == null)
                _UsersAdapter = new UsersTableAdapter();

            return _UsersAdapter;
        }
    }


    [System.ComponentModel.DataObjectMethodAttribute
        (System.ComponentModel.DataObjectMethodType.Select, true)]
    public PedChallenge.DAL.PedDataSet.UsersDataTable GetUsers()
    {
        return Adapter.GetUsers();
    }

    [System.ComponentModel.DataObjectMethodAttribute
        (System.ComponentModel.DataObjectMethodType.Select, false)]
    public PedChallenge.DAL.PedDataSet.UsersDataTable GetUserByUserID(int userID)
    {
        return Adapter.GetUserByUserID(userID);
    }

    [System.ComponentModel.DataObjectMethodAttribute
        (System.ComponentModel.DataObjectMethodType.Select, false)]
    public PedChallenge.DAL.PedDataSet.UsersDataTable GetUsersByTeamID(int teamID)
    {
        return Adapter.GetUsersByTeamID(teamID);
    }


    [System.ComponentModel.DataObjectMethodAttribute
        (System.ComponentModel.DataObjectMethodType.Select, false)]
    public PedChallenge.DAL.PedDataSet.UsersDataTable GetUsersByEmail(string Email)
    {
        return Adapter.GetUserByEmail(Email);
    }


    [System.ComponentModel.DataObjectMethodAttribute
        (System.ComponentModel.DataObjectMethodType.Insert, true)]
    public bool AddUser(int? teamID, string FirstName, string LastName, 
        string Email, string Role, int LocationID)
    {
        // Create a new UsersRow instance
        PedChallenge.DAL.PedDataSet.UsersDataTable Users = new PedChallenge.DAL.PedDataSet.UsersDataTable();
        PedChallenge.DAL.PedDataSet.UsersRow user = Users.NewUsersRow();

        if (UserExists(Users, Email) == true)
            return false;


        if (teamID == null) user.SetTeamIDNull();
        else user.TeamID = teamID.Value;
        user.FirstName = FirstName;
        user.LastName = LastName;
        user.Email = Email;
        user.Role = Role;
        user.LocationID = LocationID;

        // Add the new user
        Users.AddUsersRow(user);
        int rowsAffected = Adapter.Update(Users);

        // Return true if precisely one row was inserted,
        // otherwise false
        return rowsAffected == 1;
    }

    [System.ComponentModel.DataObjectMethodAttribute
        (System.ComponentModel.DataObjectMethodType.Update, true)]
    public bool UpdateUser(int userID, int? teamID, string FirstName, string LastName,
        string Email, string Role, int LocationID)
    {
        PedChallenge.DAL.PedDataSet.UsersDataTable Users = Adapter.GetUserByUserID(userID);
        if (Users.Count == 0)
            // no matching record found, return false
            return false;

        PedChallenge.DAL.PedDataSet.UsersRow user = Users[0];

        if (teamID == null) user.SetTeamIDNull();
        else user.TeamID = teamID.Value;
        user.FirstName = FirstName;
        user.LastName = LastName;
        user.Email = Email;
        user.Role = Role;
        user.LocationID = LocationID;

        // Update the product record
        int rowsAffected = Adapter.Update(user);

        // Return true if precisely one row was updated,
        // otherwise false
        return rowsAffected == 1;
    }

    [System.ComponentModel.DataObjectMethodAttribute
        (System.ComponentModel.DataObjectMethodType.Delete, true)]
    public bool DeleteUser(int userID)
    {
        int rowsAffected = Adapter.Delete(userID);

        // Return true if precisely one row was deleted,
        // otherwise false
        return rowsAffected == 1;
    }

    private bool UserExists(PedChallenge.DAL.PedDataSet.UsersDataTable users, string email)
    {
        // Check if user email already exists
        foreach (PedChallenge.DAL.PedDataSet.UsersRow userRow in users)
        {
            if (userRow.Email == email)
                return true;
        }
        return false;
    }
}

Some guidance in the right direction would be greatly appreciated!!

Thanks all!

Max

Answer 1

The sort of layering you're trying for usually involves moving away from the DataTable approach to something that uses an instance for (roughly) each row in the database. In other words, the DAL would return either a single User or a collection of Users, depending on which static Load method you call. This means that all of the methods that take a bunch of parameters to represent the user would instead accept a User DTO.

A DAL for users would look something like this:

public static class UserDal
{
    public static User Load(int id) { }

    public static User Save(User user) } { }

    public static IEnumerable<User> LoadByDiv(int divId) { }
}

1. It's static because it has no state. (Arguably, it could have a database connection as its state, but that's not a good idea in most cases, and connection pooling removes any benefit. Others might argue for a singleton pattern.)

2. It operates at the level of the User DTO class, not DataTable or any other database-specific abstraction. Perhaps the implementation uses a database, perhaps it uses LINQ: the caller need not know either way. Note how it returns an IEnumerable rather than committing to any particular sort of collection.

3. It is concerned only with data access, not business rules. Therefore, it should be callable only from within a business logic class that deals with users. Such a class can decide what level of access the caller is permitted to have, if any.

4. DTO stands for Data Transfer Object, which usually amounts to a class containing just public properties. It may well have a dirty flag that is automatically set when properties are changed. There may be a way to explictly set the dirty flag, but no public way to clear it. Also, the ID is typically read-only (so that it can only be filled in from deserialization).

5. The DTO intentionally does not contain business logic that attempts to ensure correctness; instead, the corresponding business logic class is what contextually enforces rules. Business logic changes, so if the DTO or DAL were burdened with it, the violation of the single responsibility principle would lead to disasters such as not being able to deserialize an object because its values are no longer considered legal.

6. The presentation layer can instantiate a User object, fill it in and ask the business logic layer to please call the Save method in the DAL. If the BLL chooses to do this, it will fill in the ID and clear the dirty flag. Using this ID, the BLL can then retrieve persisted instances by calling the DAL's Load-by-ID method.

7. The DAL always has a Save method and a Load-by-ID method, but it may well have query-based load methods, such as the LoadByDiv example above. It needs to offer whatever methods the BLL requires for efficient operation.

8. The implementation of the DAL is a secret as far as the BLL and above are concerned. If the backing is a database, there would typically be stored procedures corresponding to the various DAL methods, but this is an implementation detail. In the same way, so is any sort of caching.

Answer 2

To faciliate your design, you definitely do not want to be pulling back entire data tables and searching through them in the presentation tier. The beauty of a database is that it is indexed to facilitate fast querying of row level data (i.e. get row by indexed identifier).

Your DAL should expose a method like GetUserByUserID(int userID). You should then expose that method via the BLL, enforcing any needed business logic.

Additionally, I would stear clear of the Type Data Sets and consider an ORM tool such as Entity Framework.