I have a class with several fields.
public bool Active { get; set; }
public List<Group> Groups { get; set; }
public string FirstName { get; set; }
public string LastName { get; set; }
public Gender Gender { get; set; }
public string IdCard { get; set; }
public List<string> PhoneNumbers { get; set; }
public string Address { get; set; }
public string PicturePath { get; set; }
public string Comments { get; set; }
In the database, I have a table with corresponding fields. I want that whenever I change the value of this fields, the corresponding field in the table will be updated too automatically to the new value.
I thought maybe to implement a PropertyChanged event, and for that, I need to invoke the event handler from the set. The problem is that I read that Property getters are expected to be fast and idempotent:
Though it’s perfectly fine to iterate over an in-memory collection of objects, I wouldn’t recommend doing any kind of heavy lifting in either get or set parts. Anton Gogolev
But isn’t database updating a “heavy lifting”? Is my solution bad? What other solution would you suggest?
7
There are a few reasons against such approach.
-
Leaky abstractions. Updating a database is not a risk-free thing. Bad things happen: network connection can be dropped while you’re sending the update query; DNS server can be down; the database server can get out of space. When one calls a method such as
DAL.ProductRepository.Update(Product), the caller expects the risk of calling the method, and is expected, in turn, to catch exceptions or handle exceptional cases in any other way. On the other hand, when executingcurrentProduct.Price = 123;, it seems natural not to expect an SQL exception to be thrown, nor the main thread to be blocked for 30 seconds before the connection times out. By doing heavy database stuff in a setter, you make it easy for the caller to forget about the risks, which is not a good idea. -
Properties—would it be getters or setters—are expected to be fast, and by fast, I mean “the stuff which happens behind a getter or a setter remains relatively basic, both in terms of code and in terms of computing.” The reason for that is that callers expect them to be fast and don’t expect too much work under the hood (here again, a 30-seconds wait time before the connection times out is a good example).
-
This approach is simply uncommon, and so violates the Principle of Least Surprise. The common approach is to use whatever getter and setter, and only when all the needed changes are made to the object, pass this object to some repository, some object which owns the database context.
-
In continuity with the previous point, if the object can update the database itself, it means that the object has database context or knows the connection string and is able to create the context from that. The approach could be problematic, and it is not clear in which layer would you put such class.
However:
- You may have a very specific need when updating database through
PropertyChangedactually makes sense. I could imagine, for instance, an application configuration object which synchronizes itself with the database. Its nature is different fromProduct-style objects, which makes Principle of Least Surprise irrelevant. If class documentation is great and the culture of your team ensures that documentation will be read, the abstraction remains leaky, but the callers will be aware of the leaks before they actually happen. Again, such cases would be rare (and I haven’t seen any yet), but remain valid.