async calls inside get accessor - is it good practice?

Question

Hi!

I have a window that shows customer infomation. when the window loads, I call LoadCustomer() method from the constructor which loads the customer information from the database asynchorously, which sets the CurrentCustomer property. and then the UI is updated because it is bound to the CurrentCustomer.

    private void LoadCustomer(Guid customerID)
    {           
        CustomerContext customerContext = new CustomerContext();

        var customerQuery = customerContext.GetCustomersQuery()
                                           .Where(e => e.CustomerID == customerID);

        customerContext.Load(customerQuery, loadOperation =>
            {
                CurrentCustomer = loadOperation.Entities.SingleOrDefault();
            }, null);
    }

The senior programmer has told me that it's better to put this logic inside CurrentCustomer's get accessor, because then 1. the calls to the database will use lazy loading, and 2. the refactoring would be easier.

Is it good practice to put async database calls inside property's get accessor?

Answer 1

Generally, it's not.

Usually if getting something would involves doing something else expensive as well, you should use a full getter method just for this purpose:

public object GetSomething() { }

// accessing code
var value = obj.GetSomething();

This signifies that getting that something isn't free... Compare to:

var value = obj.Something;

This looks more like a normal value assignment hiding the dangerous async call going on behind the scene which, IMO is a bad idea.

But as with everything else... it depends on other parts in the whole architecture as well.

If the whole purpose of the object is to abstract away lots of asynchronous calls using property (such as the case with Linq2Sql or Entity Framework entities) then that's fine because you must be aware that you're dealing with calls that are not (or close to) free from the context of the code.

...

It depends on the context of the code. If whenever you are accessing this property, you are sure you'll be reminded of the expensive call, then I think it is fine. But if that isn't the case, then you should make it more explicit by turning it into a full-blown method instead of a property.

Answer 2

He is correct about it being lazy-loaded in the accessor -- since the async call won't be made on object creation. It will only be made when the accessor is called.

He is also anticipating not leaving it in the property accessor, but refactoring it elsewhere. It is easier to refactor when the call is already isolated in the accessor.

It is however, not a good idea to leave the async call in the accessor, but it doesn't sound like he is expecting it to stay there.

Answer 3

It sounds like your senior dev has missed the asynchronous nature of the code. The only way to move your code to the getter of a property would be to block the calling thread until the asynchronous operation is complete and there is a value to return. This would be very badly behaved property.