Nacharya
Nacharya

Reputation: 229

Is it a good practice to perform initialization within a Property?

I have a class PluginProvider that is using a PluginLoader component to load plugins (managed/native) from the file system. Within the PluginProvider class, there is currently defined a property called 'PluginTypes' which calls the 'InitializePlugins' instance method on get().

class PluginProvider
 {
   IEnumerable<IPluginType> PluginTypes
   {
     get
     {
        //isInitialized is set inside InitializePlugins method
        if(!isInitialized)
        {
           InitializePlugins(); //contains thread safe code
        }
        //_pluginTypes is set within InitializePlugins method
        return _pluginTypes;
     }
   }
 }

I am looking at refactoring this piece of code. I want to know whether this kind of initialization is fine to do within a property. I know that heavy operations must not be done in a property. But when i checked this link : http://msdn.microsoft.com/en-us/library/vstudio/ms229054.aspx , found this " In particular, operations that access the network or the file system (other than once for initialization) should most likely be methods, not properties.". Now I am a bit confused. Please help.

Upvotes: 3

Views: 447

Answers (4)

Eren Ers&#246;nmez
Eren Ers&#246;nmez

Reputation: 39085

  • If you want to delay the initialization as much as you can and you don't know when your property (or properties) will be called, what you're doing is fine.
  • If you want to delay and you have control over when your property will be called the first time, then you might want to make your method InitializePlugins() public and call it explicitly before accessing the property. This option also opens up the possibility of initializing asynchronously. For example, you could have an InitializePluginsAsync()that returns a Task.
  • If delaying the initialization is not a big concern, then just perform the initialization within the constructor.

Upvotes: 2

Marcel N.
Marcel N.

Reputation: 13976

What you are doing there is called lazy initialization. You are postponing doing a potentially costly operation until the very moment its output is needed.

Now, this is not an absolute rule. If your InitializePlugins method takes a long time to complete and it might impact user experience, then you can consider moving it into a public method or even making it asynchronous and call it outside of the property: at app startup or whenever you find a good moment to call a long-lasting operation.

Otherwise, if it's a short lived one-time thing it can stay there. As I said, not an absolute rule. Generally these are some guidelines for whatever applies to a particular case.

Upvotes: 1

Yuval Itzchakov
Yuval Itzchakov

Reputation: 149538

This is of course a matter of taste. But what i would do depends on the length of the operation you're trying to perform. If it takes time to load the plugins, i would create a public method which any user would need to call before working with the class. A different approach would be to put the method inside the constructor, but IMO constructors should return as quickly as possible and should contain field / property initialization.

class PluginProvider
{
    private bool _isInitialized;
    IEnumerable<IPluginType> PluginTypes { get; set;}

    public void Initialize()
    {
        if (_isInitialized)
        {
             return;
        }      

        InitializePlugins();
        _isInitialized = true;  
    }
}

Note the down side of this is that you will have to make sure the Initialize method was called before consuimg any operation.

Another thing that just came to mind backing this approach is exception handling. Im sure you wouldn't want your constructorcto be throwing any kind of IOException in case it couldn't load the types from the file system.

Upvotes: 1

fotijr
fotijr

Reputation: 1096

Any initialization type of code should be done in the constructor, that way you know it will be called once and only once.

public class PluginProvider
{
   IEnumerable<IPluginType> PluginTypes
   {
      get
      {
        return _pluginTypes;
      }
   }

   public PluginProvider()
   {
      InitializePlugins();
   }
}

Upvotes: 1

Related Questions