8

Some of the answers and comments to this question: Simplest C# code to poll a property?, imply that retrieving data from a database in a property's getter is Generally a Bad Idea.
Why is it so bad?

(If you have sources for your information, please mention them.)

I will usually be storing the information in a variable after the first "get" for reuse, if that influences your answer.

Community
  • 1
  • 1
Protector one
  • 6,926
  • 5
  • 62
  • 86
  • Since the purpose of a getter is to reflect the current state of the object, retrieving the value from the DB is an inappropriate action at this time. Object instantiation or lookup is the correct time to access the DB. Then, the getter can return the value that was read. – KevinDTimm Jul 13 '11 at 17:21
  • @KevinDTimm But what if I don't know if I'll need that data from the DB at initialization-time? – Protector one Jul 13 '11 at 17:24

7 Answers7

12

Because retrieving data from a database could cause any number of exceptions, and property getters, as a rule, should never throw exceptions.

The expected behavior of a property getter is just to return a value; if it's actually doing a lot more than that, it should be a method.

Microsoft's guide for Property Design explains the reasons: https://learn.microsoft.com/en-us/dotnet/standard/design-guidelines/property

Flynn1179
  • 11,925
  • 6
  • 38
  • 74
  • 1
    Ok, why shouldn't they throw exceptions? – Protector one Jul 13 '11 at 17:19
  • 3
    One reason that they shouldn't be throwing exceptions is because properties can be part of databinding in a UI. I'm sure there are many other reasons why, though. – Dismissile Jul 13 '11 at 17:21
  • 1
    They shouldn't throw exceptions as a side-effect of the fact that they generally shouldn't contain logic in the first place. – Dave Swersky Jul 13 '11 at 17:22
  • Because the job of a getter is generally to return a stored value...not to execute a bunch of logic. In .NET, users of your library will not expect a property to throw exceptions. – alexD Jul 13 '11 at 17:24
  • @Dave Any sources for this? I looked for something like this, but couldn't find it at MSDN, at least. – Protector one Jul 13 '11 at 17:25
  • 2
    @Protector: You're delving into aspects of best practices, which are not documented in MSDN the same way as are language syntax and object namespaces. You might find something here: http://stackoverflow.com/questions/1529604/c-antipatterns – Dave Swersky Jul 13 '11 at 17:28
  • Properties can be used by Visual Studio, and thus if you put a query in a getter it might be executed during editing. I just corrupted a project in that way, I had to edit cs files manually and regenerate the application to put it back to work. A huge pain for such a lovely Friday. – Grigory Kornilov Apr 18 '14 at 13:02
6

It's bad because (among other things) it violates the Principle of Least Astonishment.

Programmers generally expect properties to do simple gets/sets. Encapsulating data access in a property, which could throw exceptions, cause side effects, and change the state of the data in the database, is not what is generally expected.

I'm not saying there is no case for complex properties - sometimes, it can be a good solution. But, it is not the expected way to do things.

womp
  • 115,835
  • 26
  • 236
  • 269
  • This is correct, but to be fair to a newbie, it is [begging the question](https://en.wikipedia.org/wiki/Begging_the_question). Programmers expect it because it shouldn't be done and it shouldn't be done because programmers expect it. But in the end, you just have to accept that it is convention. – xr280xr Nov 12 '22 at 19:17
6

The Short Version: Making a property getter directly access a database would violate The Separation of Concerns principle.

More Detail: Generally speaking, a property is intended to represent data associated with an object, such as the FirstName property of a Person object. Property values may be set internally or externally, but the act of modifying and retrieving this data on the object should be separated from the act of retrieving or committing that data to a permanent store.

Dave Swersky
  • 34,502
  • 9
  • 78
  • 118
3

Any time you accessed the getter, you'd be making another call out to your database.

AllenG
  • 8,112
  • 29
  • 40
  • What if I store the answer after the first get, in a singleton-pattern-like scenario? – Protector one Jul 13 '11 at 17:20
  • 1
    Then why not just make a method to populate your class from a database? Then the class itself never has to worry about your databases at all. – AllenG Jul 13 '11 at 17:22
  • Then you should move the logic into a private method within the class and store the data in a private field that your getter provides access to. The logic does not need to be in the property itself. – alexD Jul 13 '11 at 17:25
  • 1
    +1 We had a similar situation in which such property was used in a .OrderByDescending(o => o.PropertyThatDoesMoreThanYouThink) method on a collection of several hundreds of objects. Developers sorted on this property without ever assuming that this property accesses database. Of course, this was latter fixed beacuse it caused hundreds of unnecessary database queries and performance problems. – Ivan Golović Oct 17 '14 at 11:21
3

A getter, by definition, should just be encapsulating data, not functionality.

Also, you would likely be redefining functionality and making many trips to the database if you had more than one getter that needed to round-trip to the database. Why not handle that in a centralized place rather than splitting that out among multiple properties?

Methods are for class-level data modification and functionality, properties are for individual data modification and retrieval only.

Jordan
  • 31,971
  • 6
  • 56
  • 67
2

Besides exceptions being likely, querying a database is a slooooow operation. In both aspects, using property getters as a database accessor violates the principle of least astonishment to client code. If I see a library class with a property, I don't expect it to do a lot of work, but to access a value that's easily retrieved.

The least astonishing option here is to provide an old fashioned, simple Get function.

Pontus Gagge
  • 17,166
  • 1
  • 38
  • 51
1

If you are indeed only retrieving the value from the database and not also writing its value back, such as a read only property, there is nothing inherently wrong. Especially if the property cannot exists without its parent. However in implementation it can cause maintainability problems. You are coupling the process of retrieving stored information with access to that information. If you continue to follow this pattern for other properties, and aspects of your storage system change, the change could proliferate throughout your code base. For example a table name or column data type change. This is why it is bad to have database calls in your property getter.

On a side note: if the database is throwing exceptions when you try to retrieve the value, then obviously there is a bug in your code (or the calling client's code) and the exception will still surface regardless of where you put the data access code. Many times data is backed by some sort of collection within the class that can throw exceptions and it is standard practice to store property values in this manner (see EventHandlerList).

Properties were designed specifically because programmers need to perform additional logic when getting and setting values, such as validation.

With all of that having been said, reexamine your code and ask yourself "how easy will this be to change later?" from there you should be on your way to a more maintainable solution.

Charles Lambert
  • 5,042
  • 26
  • 47