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

Educational website using ASP.NET webforms

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

Problem

Our team is developing an educational website using ASP.NET webforms. We designed DAL & BAL classes and now I have some questions:

-
Are DAL & BAL designed well?

-
Would it be better using ORM (like EF) instead of using our own classes? (ORM is slow, but I'm not sure)

-
Should we ignore BAL and only have DAL (2 layers instead of 3 layers)?

Notice that:

  • there are many users and hits.



  • every page needs a database and there are many queries.



  • performance and speed are important for our team.



DAL

```
// These lines will import the needed namespaces
using System;
using System.Data;

// Declare our datalayer namespace
namespace DataLayer
{

#region "Cat_Table Class"
// Create the class, based on our table and make it Serializable()
// Making it Serializable means that we will be able to store an instance
// of our class in a session variable or save and load it from disk
[Serializable()] public partial class Cat_Table
{

#region "Instance Variables"
protected int m_CatID;
protected string m_CatName;
// The IsDirty flag is set whenever a property is changed
protected bool m_IsDirty;
// The m_ConnectionString holds the connection string
protected string m_ConnectString;
// The m_IsUpdate determines if, when we save a record
// it is an insert or update
protected bool m_IsUpdate;

#endregion

#region "Constructors"
// This is the default empty constructor. Basically
// all it does is setup the connectionstring
public Cat_Table()
{
// Call SetConnectionString() to set our connection string
// for connecting to the database
SetConnectString();
// The m_IsUpdate flag holds weather we are doing an insert or an update
// Since no primary key was passed, this is not an update. Its an insert.
m_IsUpdate = false;
}

// This is the default constructor called when passing a primary key.
// It basically does the same thing as the empty constructor, but it

Solution

private class Comp_CatName_D : IComparer
{

    public int Compare(object x, object y)
    {
        BusinessLayer.Cat_Table Cat_Table1 = (BusinessLayer.Cat_Table)x;
        BusinessLayer.Cat_Table Cat_Table2 = (BusinessLayer.Cat_Table)y;
    }
}


You see how ugly this is. Use the generic IComparer so you can have strongly typed comparisons which don't require casts.

MSDN

try {
    return Cat_Table1.CatName.CompareTo(Cat_Table2.CatName);
} catch (Exception) {
    return 0;
}


If something went wrong so badly it throws an exception, we'll just pretend they're the same? This is logically incorrect and will cause very hard to track down bugs.

try {
    j = Cat_Table1.CatName.CompareTo(Cat_Table2.CatName);
    if (j==1) return -1;
    if (j==-1) return 1;
} catch (Exception) {
    j = 0;
}
return j;


Also known as

return -Cat_Table1.CatName.CompareTo(Cat_Table2.CatName);


class Cat_Tables


Classes use UpperCamelCase as naming convention.

foreach (BusinessLayer.Cat_Table Cat_TableObj in this.List)


use more descriptive names. This will turn the previous line of code in this:

foreach(var table in tables)


public enum SortFields
{
    Sort_CatID,
    Sort_CatName
}


Enums are UpperCamelCase as well.

protected int m_CatID;


No hungarian notation!

Last but not least:

USE AN ORM

Getting a proper ORM up and running is very hard to get right and by creating your own you are

  • Introducing buggy code



  • Using a very limited subset of a full blown ORM



  • Losing performance (yes, losing): you're missing out on caching and other optimization techniques



  • Hardcoding the database dependencies

Code Snippets

private class Comp_CatName_D : IComparer
{

    public int Compare(object x, object y)
    {
        BusinessLayer.Cat_Table Cat_Table1 = (BusinessLayer.Cat_Table)x;
        BusinessLayer.Cat_Table Cat_Table2 = (BusinessLayer.Cat_Table)y;
    }
}
try {
    return Cat_Table1.CatName.CompareTo(Cat_Table2.CatName);
} catch (Exception) {
    return 0;
}
try {
    j = Cat_Table1.CatName.CompareTo(Cat_Table2.CatName);
    if (j==1) return -1;
    if (j==-1) return 1;
} catch (Exception) {
    j = 0;
}
return j;
return -Cat_Table1.CatName.CompareTo(Cat_Table2.CatName);
class Cat_Tables

Context

StackExchange Code Review Q#91004, answer score: 7

Revisions (0)

No revisions yet.