Is using property getters for initialization (to avoid having to call methods in specific order) bad practice?

Question

Suppose I have a class that provides some data to my application. Data initially comes from database, and I provide it through some methods that handle the whole database thing and present the result as a usable class instead of raw query result. This class has to do some setup (not complex) to make sure any method called can use the database (e.g. connect to database, make sure it contains some critical info, etc). So, were I to put it in a method (say, method Init(), that would handle checking for database, connecting to it, verifying that it does contain the info), I would have to make sure that this method is called before any other method. So, I usually find that instead of doing this:

public class DataProvider
{
    private SqlController controller;

    public void Init()
    {
        controller = new SqlController();
        controller.Init();
        controller.ConnectToDataBase();
        CheckForCriticalInfoInDatabase();
    }

    public Data GetData()
    {
        // get data from database (not actually going to use raw queries like that, just an example)
        var queryResult = sqlController.RunQuery("SELECT something FROM SOME_TABLE");
        // and present it as usable class
        Data usefulData = QueryResultToUsefulData(queryResult);
        return usefulData; 
    }

    ...
}

and then always making sure I call Init() before GetData(), i do something like

    private SqlController _controller;

    private SqlController controller
    {
        get
        {
            if (_controller == null)
            {
                _controller = new SqlController();
                _controller.Init();
                _controller.ConnectToDataBase();
                CheckForCriticalInfoInDatabase();
            }
            return controller;
        }
    }

So, now i can be sure that i won't use an uninitialised SqlController, and I don't have to do that same null check in every method that uses it. However, I never noticed getters being used this way in other peoples' code.

Is there some pitfall I don't see? To me it looks like it's the same as lazy initialization, with the exception being that I use it not because the initialization is heavy or long, but because I don't want to check the order in which I call methods. This question points out that it's not thread-safe (not a concern in my case, plus I imagine it could be made thread-safe with some locks) and that setting the property to null will result in unintuitive behaviour (not a concern, because I don't have a setter at all and the backing field shouldn't be touched either way).

Also, if this kind of code IS bas practice, what is the proper way to ensure that my methods don't rely on order in which they are called?

Connecting to the database within a property is a bad idea. Properties should be very quick, whereas connecting to a database could potentially take several seconds on a slow connection, not to mention the additional checks you're doing immediately after connecting. `SqlController` should be responsible for doing those things upon first use of one of its methods. If you had to pass parameters to these methods, they should be passed to the constructor and stored until they're needed by those methods. — madreflection, Feb 21 '20 at 00:04
"properties should be very quick" - could you explain why? Why should a private property be quick when a method has no such constraint? Is there some expectation placed on a property? @madreflection — defaultUsernameN, Feb 21 '20 at 00:10
I like to think of Properties as they are often called...getters and setters. If you want to "do" something, call a method. I wouldn't say its wrong to do what you are doing, its just not the accepted "norm" ... In regards to thread safety, you could make it thread safe by using a "lock" if you wanted to but as you've said, you dont need to. — Andrew Harris, Feb 21 '20 at 00:14
[Choosing Between Properties and Methods](https://learn.microsoft.com/en-us/previous-versions/dotnet/netframework-4.0/ms229054(v=vs.100)). Use a method when the "operation is orders of magnitude slower than a field set would be." Connecting to a database takes an unpredictable amount of time so prepare for the worst case in your design. — madreflection, Feb 21 '20 at 00:16
So, would just rewriting this with a method private SqlController GetSqlController() (which would still use that same backing field) instead of property getter make it more like the norm? Wouldn't that just be undoing the syntactic sugar? — defaultUsernameN, Feb 21 '20 at 00:20
You'd be undoing more than the syntactic sugar. You'd be undoing the expectation that it's just returning an attribute of the object, so when connecting does take too long or throws an exception ([which a getter *should* avoid doing](https://learn.microsoft.com/en-us/dotnet/standard/design-guidelines/property) unless internal state is invalid), it's from a reasonable place. — madreflection, Feb 21 '20 at 00:25
Thanks for the link. Still, say I was not acessing a database, but instead doing something quick like setting up some fields - would that still belong in a method and not a property getter? My concern is that waiting for first use of a method forces me to insert some kind of initialization check into each and every public method, which seems counterproductive. @madreflection — defaultUsernameN, Feb 21 '20 at 00:25
In that case, rename `ConnectToDataBase` to something else. Either way, though, that responsibility should be placed on `SqlController`, not its consumer. The property should assign `_controller` if it hasn't already and then return it. Done. — madreflection, Feb 21 '20 at 00:26
Okay, got it - getters are for returning values) So what would be the alternative? Say I need some fields set up in the class itself (DataProvider class, not SqlController) and I want to delay that setup till the first public method call, but don't want to repeat the same "if (someField == null) SetFields in every method? With the property i had a built-in way to achieve that - can i replicate that? Using a constructor won't always work, I imagine - what if fields are set depending on current state of some other class, or DataController is a static class? @madreflection — defaultUsernameN, Feb 21 '20 at 00:38
*"don't want to repeat the same ..."* - I think you're placing that constraint on yourself too soon. While you're learning how to structure the code, don't be afraid to repeat yourself. Once you have something that's working, refactor that repetition to be consistent, then refactor the repeated parts to methods that you call instead. It's difficult to plan for that if you're trying to solve another problem at the same time. — madreflection, Feb 21 '20 at 00:49
Having said all that, I think you need to "take it offline" and do some critical thinking about both your design and what you've learned. That may lead to another question framed from your new perspective, and answers would be more targeted to that. Meanwhile, you should consider accepting Slipoch's answer. — madreflection, Feb 21 '20 at 00:53

score 0 · Accepted Answer · answered Feb 21 '20 at 00:24

As @madreflection said in the OP comments, use a method for anything that is possibly going to be slow. Getters and setters should just be quick ways of getting and setting a value.

Connections to dbs can be slow or fail to connect so you may have catches setup to try different connection methods etc.

You could also have the checking occur in the constructor of the object, that way the object cannot be used without init() being run in a different function, saving on time tracing where an error is actually occurring.

For example if you had one function create the object, do a bunch of 'stuff' then try to use the object without running init(), then you get the error after all of the 'stuff' not where you created the object. This could lead you to think there is something wrong in whatever way you are using the object, not that it has not been initialised.

Thanks! Got it about database, shouldn't be doing complex stuff in a getter. What if the checking was something that should occur not at the time of construction, but at the time first public method is called? Is there a way for me to replicate current behaviour (initialisation occurs when first needed, and I don't have to manually check if it occured) without breaking guidelines? — defaultUsernameN, Feb 21 '20 at 00:44
, I suppose you could have a bool inside the object, then each function could check that bool and if it has not been checked run the check, but you do still run into the same issue where something in the constructor that is incorrect may cause the issue later. — Slipoch, Feb 24 '20 at 02:26

Is using property getters for initialization (to avoid having to call methods in specific order) bad practice?

1 Answers1