3 layers architecture in C#

Question

I will build a system in C# and I want to use the three layers architecture is next:

Entity Layer

namespace EntitiesLayer
{
    public class ECategoria
    {
        private int _idcategoria;
        private string categoria;  
        private string descripcion;
    public int Idcategoria
    {
        get { return _idcategoria; }
        set { _idcategoria = value; }
    }

    public string Categoria
    {
        get { return categoria; }
        set { categoria = value; }
    }

    public string Descripcion
    {
        get { return descripcion; }
        set { descripcion = value; }
    }

Data Access Layer

public class DCategoria
{
    public string Act_Categoria(ECategoria categoria)
    {
        string rpta = "";
        DataSet ds = new DataSet();

        try
        {
        string query = "SP_ACTCATEGORIA";
        Dictionary<string, object> parameters = new Dictionary<string, object>();
        parameters.Add("@idcategoria", categoria.Idcategoria);
        parameters.Add("@categoria", categoria.Categoria);
        parameters.Add("@descripcion", categoria.Descripcion);
        ds = dbconnection.execute_query(query, parameters);
        if (ds == null)
        {
            rpta = "Error Insertando";
        }
        else
        {
            rpta = "REGISTRO REGISTRADO/ACTUALIZADO EXITOSAMENTE";
        }
    }
    catch (Exception ex)
    {
        rpta = ex.Message;
    }

    return rpta;
}

Business Logic Layer

public class BCategoria { private DCategoria data_categoria = new DCategoria();

public string Act_categoria(ECategoria categoria)
{
    return data_categoria.Act_Categoria(categoria);
}

public DataSet Select_categoria()
{
    return data_categoria.Select_Categoria();
}

public string Delete_categoria(ECategoria categoria)
{
    return data_categoria.Delete_Categoria(categoria);
}

public DataSet FilterbyID(ECategoria categoria)
{
    return data_categoria.FilterbyID(categoria);
}

public DataSet FilterByName(ECategoria categoria)
{
    return data_categoria.FilterByName(categoria);
}

Presentation Layer

public ECategoria get_data()
{
    ECategoria categoria = new ECategoria();
    categoria.Idcategoria = Convert.ToInt32(this.txtcodigo.Text.Trim());
    categoria.Categoria = this.txtcategoria.Text.Trim();
    categoria.Descripcion = this.txtcategoria.Text.Trim();
    return categoria;
}

private void btnNuevo_Click(object sender, EventArgs e)
{
    categoria = get_data();
    rpta = bus_categoria.Act_categoria(categoria);
}

Is this a good implementation of the three layers architecture?

Answer 1

Few notes about naming conventions (I do not repeat for each method/class but they apply to all of them).

Namespace EntitiesLayer does not help to categorize your code. Namespace are hierarchical and here you're not using them in that way. Obviously it's not mandatory but something like Juan.AwesomeProduct.Data, Juan.AwesomeProduct.Model and Juan.AwesomeProduct.Presentation may help you to organize your code. I am shameless and I admit that at the very beginning (with Managed Extensions for C++, you're lucky if you never had to use it) I used namespaces like you're doing and I still regret.

Classes like ECategoria should not be prefixed with E (or C or anything else.) That's what namespaces are for.

Idcategoria is a public property then it should be PascalCase IdCategoria.

_idcategoria is underscore prefixed but descripcion and categoria are not. Pick a convention (with or without prefix) but be consistent. There are good reasons both to use it or to avoid it, have an informed decision and don't look back.

You're not validating any value in your entities layer. Is acceptable to have null for Descripcion? A negative ID? A blank string?

I see no reason for ECategoria to do not be sealed.

Also take care of indentation.

DCategoria (again, remove that prefix) is an utility class. More often than not you do not need it. Its responsibility may be moved to another class that does something else. It may also be a generic class for many entities. Something like a repository, reconsider that name. Also consider to make it sealed (do you want to extend it through inheritance? Where extension points are?)

Usually underscore is not used to separate words, Act_Categoria() should be PascalCase (it's a public method) then ActCategoria() may be more appropriate. However you're mixing two languages. Use English or your own language but do not mix them in code. If you can you may write everything in English and keep comments in your own language (if you work in an company they should have a procedure for this.)

rpta and ds are declared at the beginning of your method. In C# you can (and you should) declare them as late as possible. Ideally in the same moment you initialize them. Let's see a proof of concept which also includes some others tips:

public string Act_Categoria(ECategoria categoria)
{
    try
    {
        var parameters = new Dictionary<string, object>();
        parameters.Add("@idcategoria", categoria.Idcategoria);
        parameters.Add("@categoria", categoria.Categoria);
        parameters.Add("@descripcion", categoria.Descripcion);
        const string query = "SP_ACTCATEGORIA";
        var ds = dbconnection.execute_query(query, parameters);

        return ds == null ? "Error Insertando" : "REGISTRO REGISTRADO/ACTUALIZADO EXITOSAMENTE";
    }
    catch (Exception ex)
    {
        return ex.Message;
    }
}

Note that you're catching Exception. It's a very bad thing to do (I won't repeat here the reasons, do a quick search of check any of my previous reviews here.) Catch the exceptions you know may happen and you want to ignore. NullReferenceException is a programming error, not a run-time transitory error condition you may want to swallow.

In your business layer data_categoria may be readonly. In this moment and with this code I do not see any reason to have a BCategoria class. It just forwards calls to DCategoria. Drop one of them until you will actually need it. Business logic should add some value, what's the value of a tunnel? You still expose the same model (ECategoria) and you just forward calls. In a somehow simple scenario you may merge business layer and data layer but when things are more complex the business layer should have its own model (it is the model...) I'd suggest to do not make things complicate if unnecessary, maybe a 2-tiers architecture is what you should start with.

Presentation layer is just a proof of concept however (besides naming convention) it's where you should put more logic. Business layer and Presentation Layer are what will change more often (and probably independently) but here you're directly calling business layer without any abstraction. How do you test your user interface without the attached business layer? What will happen when you need, for example, a IsChanged property in ECategoria? Will you add it in the shared class even if it's used only in presentation? Check MVC, MVP and MVVM to pick the one that best suits your needings (and your UI framework).

Also note that this line:

categoria.Idcategoria = Convert.ToInt32(this.txtcodigo.Text.Trim());

May fail (and trimming is optional here.) User input may be wrong and you must handle that, proof of concept:

int categoryId;
if (Int32.TryParse(this.txtCodigo.Text, out categoryId))
    categoria.CategoryId = categoryId;
else
    ; // Ooops input is not a number (formatted according to current culture)