Advice for this code ( defining a simple class in C# )

Question

Can you give advice if this is right or wrong?

public class DAL 
{
    Members member;

    public DAL(){ }

    public DAL(Members mmber)
    {
        member = mmber;
    }
}

Answer 1

This is fine, as long as you aren't going to try to call a method on members when it is null (with the first constructor).

Without more context it is difficult to say anything else. I can only assume that you have a reason to provide a public, no-arg constructor, and that the member field can be set by some other mechanism.

Answer 2

You're on the right track -- there's nothing really wrong with it. Consider giving your "Members" variable name something that helps identify it as a member field in your function, such as m_members. That way you'll be able to use variable names like member or members in the methods in this class.

Answer 3

I would change the overloaded constructor to this:

public DAL(Members member)
{
    this.member = member;
}

The this in the constructor allows you to use the member field in the class and this way you don't have to rename your parameters to something that doesn't look right.

Answer 4

Yeah, it's fine.

I would add private to the attribute and remove the no arg constructor:

public class DAL 
{
     private Members member;

     // public DAL(){ } forces DAL always have a member value

    public DAL(Members member)
    {
        this.member = member;
    }
}

But I wouldn't say it is "wrong" if you don't do it.

Answer 5

One tip - you should chain your constructors together:

public DAL()
{
}

public DAL(Members mmber)
    : this()
{
    member = mmber;
}

No impact on the current code, but chaining is important when you get some more logic in your constructors.