Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Is it a bad idea to create directories in the constructor?

At first it seemed natural - if a set of directories didn't exist, the object that works on them wouldn't be able to fulfill its contracts. So in the constructor I have some logic to check if some directories exist and create them if not. Though not actually a singleton yet, this object is used like one.

Is the constructor a bad place for this kind of setup logic?

Background
The class is called FileGetter. It abstracts getting particular files from a remote server, extracting them, and preparing the files and placing them in another directory where a second class will be filesystemwatching/processing the data.

like image 748
HAL9000 Avatar asked Dec 02 '22 02:12

HAL9000


2 Answers

From an Inversion of Control or Dependency Inversion perspective, yes, it's incorrect.

You state that the object that works on the directories couldn't do it's work if they didn't exist. I'd abstract out the providing and the check/creation of the directories to another abstraction and then pass the implementation of that abstraction to your object.

Then, your object would simply get the directories from this abstraction and continue from there.

As an example, here's what I mean. First, there is the abstraction of the directory provider, like so:

public interface IDirectoryProvider
{
    // Gets the full paths to the directories being worked on.
    IEnumerable<string> GetPaths();
}

Then there is the implementation.

public sealed class DirectoryProvider
{
    public DirectoryProvider(IEnumerable<string> directories)
    {
        // The validated directories.
        IList<string> validatedDirectories = new List<string>();

        // Validate the directories.
        foreach (string directory in directories)
        {
            // Reconcile full path here.
            string path = ...;

            // If the directory doesn't exist, create it.
            Directory.CreateDirectory(path);

            // Add to the list.
            validatedDirectories.Add(path);
        }
    }

    private readonly IEnumerable<string> _directories;

    public IEnumerable<string> GetPaths()
    {
         // Just return the directories.
         return _directories;
    }
}

Finally, there is your class which processes the directories, which would look like this:

public sealed DirectoryProcessor
{
    public DirectoryProcessor(IDirectoryProvider directoryProvider)
    {
        // Store the provider.
        _directoryProvider = directoryProvider;
    }

    private readonly IDirectoryProvider _directoryProvider;

    public void DoWork()
    {
        // Cycle through the directories from the provider and
        // process.
        foreach (string path in _directoryProvider.GetPaths())
        {
            // Process the path
            ...
        }
    }
}
like image 93
casperOne Avatar answered Dec 06 '22 11:12

casperOne


I would say that it depends. In general it's a good idea to make object construction as cheap as possible; that is, have the constructors contain the least possible amount of logic. That speaks against creating directories in the constructor. If, on the other hand, the object really cannot operate at all without the directories, it might be a good idea to fail as early as possible (for instance in case the directories cannot be created for some reason). This could speak for creating them in the constructor.

Personally I would probably lean towards not creating them in the constructor, but instead have each method that needs them call some method that creates the directories, if that is not already done.

like image 23
Fredrik Mörk Avatar answered Dec 06 '22 11:12

Fredrik Mörk