Is it ok to use C# Property like this

Question

One of my fellow developer has a code similar to the following snippet

class Data
{
     public string Prop1
     {
           get
           {
                // return the value stored in the database via a query
           }
           set
           {
                // Save the data to local variable
           }
     }

     public void SaveData()
     {
          // Write all the properties to a file
     }

}

class Program
{
    public void SaveData()
    {
         Data d = new Data();
         // Fetch the information from database and fill the local variable
         d.Prop1 = d.Prop1; 
         d.SaveData();
    }
}

Here the Data class properties fetch the information from DB dynamically. When there is a need to save the Data to a file the developer creates an instance and fills the property using self assignment. Then finally calls a save. I tried arguing that the usage of property is not correct. But he is not convinced.

This are his points

1. There are nearly 20 such properties.
2. Fetching all the information is not required except for saving.
3. Instead of self assignment writing an utility method to fetch all will have same duplicate code in the properties.

Is this usage correct?

Answer 1

I don't think that another developer who will work with the same code will be happy to see :

d.Prop1 = d.Prop1; 

Personally I would never do that.

Also it is not the best idea to use property to load data from DB. I would have method which will load data from DB to local variable and then you can get that data using property. Also get/set logically must work with the same data. It is strange to use get for getting data from DB but to use set to work with local variable.

Answer 2

Properties should really be as lightweight as possible.

When other developers are using properties, they expect them to be intrinsic parts of the object (that is, already loaded and in memory).

The real issue here is that of symmetry - the property get and set should mirror each other, and they don't. This is against what most developers would normally expect.

Having the property load up from database is not recommended - normally one would populate the class via a specific method.