CODEWITHSUNDEEP
c#.net-3.5
oshirowanen
oshirowanen

Reputation: 15965

Public fields are rarely a good idea?

Apparently public fields are rarely a good idea and add nothing over properties.

For example:

public class ClientGroupDetails
{
    public DateTime Col2;
    public String Col3;
    public Int32 Col4;

    public ClientGroupDetails(DateTime m_Col2, String m_Col3, Int32 m_Col4)
    {
        Col2 = m_Col2;
        Col3 = m_Col3;
        Col4 = m_Col4;
    }

    public ClientGroupDetails() { }
}

[WebMethod()]
public List<ClientGroupDetails> GetClientGroupDetails(string phrase)
{
    var client_group_details = new List<ClientGroupDetails>();

    using (connection = new SqlConnection(ConfigurationManager.AppSettings["connString"]))
    {
        using (command = new SqlCommand(@"select col2, col3, col4 from table1 where col1 = @strSearch", connection))
        {
            command.Parameters.Add("@strSearch", SqlDbType.VarChar, 255).Value = phrase;

            connection.Open();
            using (reader = command.ExecuteReader())
            {
                int Col2Index = reader.GetOrdinal("col2");
                int Col3Index = reader.GetOrdinal("col3");
                int Col4Index = reader.GetOrdinal("col4");

                while (reader.Read())
                {
                    client_group_details.Add(new ClientGroupDetails(
                        reader.IsDBNull(Col2Index) ? (Nullable<DateTime>)null : (Nullable<DateTime>)reader.GetDateTime(Col2Index),
                        reader.IsDBNull(Col3Index) ? null : reader.GetString(Col3Index),
                        reader.GetInt32(Col4Index)));
                }
            }
        }
    }

    return client_group_details;
}
}

My question is, how do I convert this code which uses public fields to properties?

Upvotes: 3

Views: 123

Answers (3)

MoonKnight
MoonKnight

Reputation: 23831

You can use private and public key words to provide access to these variables. The full (non-lazy) way is (where here T is some class type) 

private T myT;
public T MyT
{
    get { return this.myT; }
    set { this.myT = value; }
}

or for the lazy of us, you can use 

public T MyT { get; set; }

I hope this helps.

Upvotes: 2

Reed Copsey
Reed Copsey

Reputation: 564851

The simplest way is to just do:

public DateTime Col2 { get; set; }
public String Col3 { get; set; }
public Int32 Col4 { get; set; }

This will make them auto-implemented properties instead of fields.

Note that I do disagree with your statement that "public fields are never a good idea" - I do agree that they are rarely a good idea, but there are (rare) times when they are far nicer to use than public properties. For some good arguments on when public fields may be appropriate, I'd recommend reading Rico Mariani's "Ten Questions on Value-Based Programming", both the questions as well as the answers. He makes a strong case for public fields, especially if there are no illegal values and you expect the common use case to include frequent mutation.

That being said, I do think, in your specific case, converting to properties would be beneficial. In addition, I would recommend converting this to properties with meaningful names (not Col2, but something more like Date, etc).

Edit in response to comments:

Your full class would become:

public class ClientGroupDetails
{
    public DateTime Col2 { get; set; }
    public String Col3 { get; set; }
    public Int32 Col4 { get; set; }

    public ClientGroupDetails(DateTime m_Col2, String m_Col3, Int32 m_Col4)
    {
        Col2 = m_Col2;
        Col3 = m_Col3;
        Col4 = m_Col4;
    }

    public ClientGroupDetails() { }
}

However, I would recommend reworking this to have better names, ie:

public class ClientGroupDetails
{
    // Not 100% sure what appropriate names should be here
    public DateTime Date { get; set; }
    public String Name { get; set; }
    public Int32 Id { get; set; }

    public ClientGroupDetails(DateTime date, String name, Int32 id)
    {
        this.Date = date;
        this.Name = name;
        this.Id = id;
    }

    // I also wouldn't include this unless you really need it...
    // public ClientGroupDetails() { }
}

Note that, if you aren't going to edit the values after you create this class, you can make them properties that are publically visible to get, but not to set, via:

    public DateTime Date { get; private set; }
    public String Name { get; private set; }
    public Int32 Id { get; private set; }

This will allow your class (ie: the constructor) to set the values, but nothing outside the class will be allowed to change them.

Upvotes: 11

Cole Cameron
Cole Cameron

Reputation: 2233

Here's an example of property syntax. Within the get and set blocks, you can now execute code like a function, so you could validate the value before assigning it to the private variable. You could also return a calculated value from the get block rather than just exposing a private value.

private MyType _myField;

public MyType MyField
{
    get { return _myField; }
    set { _myField = value; }
}

There is also the shorthand, if you're just hiding a private member:

public MyType MyField { get; set; }

Upvotes: 1

Related Questions