HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

3-layer ASP.NET app - Need critique/advice

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
layercritiqueneedappnetadviceasp

Problem

My current setup is a 3 layered application: UI > BLL > DAL. Each layer is set up as a separate project. The UI is an ASP.Net website and the BLL & DAL are Class Library projects.

I've shown some sample code from each of my layers. Hopefully you guys can critique my application design and give me some tips/pointers or I'm doing this wrong, steer me in the right direction. Thanks!

DAL Method:

public static object Get_UserId(string username)
{
    ConnectionSettings connectionSettings = new ConnectionSettings();
    SqlConnection sqlConnection = new SqlConnection(connectionSettings.ConnectionString);
    string sql = String.Format("select UserId from Users where Username = '{0}'", username);
    try
    {
        sqlConnection.Open();
        SqlCommand sqlCommand = new SqlCommand(sql, sqlConnection);
        return sqlCommand.ExecuteScalar();
    }
    catch
    {                
        throw new DALException("Unable to retreive User ID");
    }
    finally
    {
        sqlConnection.Close();
    }
}


The BLL method responsible for retreiving User ID:

public static int GetUserId(string username)
 {
     try
     {
         int userId = Int32.Parse(UserDAL.Get_UserId(username).ToString());
         return userId;
     }
     catch (Exception ex)
     {
         throw new BLLException(ex.Message);
     }
 }


The call to the BLL from UI:

try
{
    int id = UserBLL.GetUserId("bobloblaw");
    Response.Write(id);
}
catch (DALException dalEx)
{
    Response.Write(dalEx.Message);
}
catch (BLLException bllEx)
{
    Response.Write(bllEx.Message);
}

Solution

Things I would change in your code/approach:

  • log errors



  • get rid of static service methods



  • hide dll exceptions in ui. Catch it in BLL and log it/do something with it.



  • change DLL so the method in BLL instead of:



int userId = Int32.Parse(UserDAL.Get_UserId(username).ToString());

would use:

int userId = new UserDAL().Get_UserId(username);

These are first things that come to my head.

Context

StackExchange Code Review Q#3312, answer score: 2

Revisions (0)

No revisions yet.