I am thinking if it’s a good practice to have classes that have Open and Close methods

Not in general. But in this example, where a class creates an abstraction for some file writer or stream writer service, where the terms Open and Close are well-established and popular, such methods make sense. It will allow you to create an CsvWriterService with a specific combination of IFileSystem and IStreamReaderWrapperFactory and then process multiple files with that combination. This can be useful regardless whether ones uses an IoC container for it, or does Dependency Injection manually.

What you have to care for, however, is the temporal coupling and the possibility of someone calling the methods in the wrong order. For example, your current implementation misses preventive measures against someone calling Open twice, without calling Close in between.

For dealing with this, I would probably set _csvReader and _streamReader to null inside Close after disposing them. That will allow to check if those variables are not null inside Open, and when not, either call Close there, or throw a specific exception. There may also be other operations in the CsvWriterService which will not work as long as the file stream is not in the “opened” state. They all should check this and throw a specific exception as well.

Can it be benefical to omit Open / Close and don’t separate construction and opening? That is somewhat opinionated and may depend on the specific case. When the design using Open / Close will force you to write a lot of boiler plate code inside where you have to check for the Open state in several methods and operations, then the answer may be “yes”. However, this comes for a price to pay: as mentioned in a comment below your question, this design will probably require an extra factory class in case you need to process multiple files with the same pair (IFileSystem, IStreamReaderWrapperFactory), and that’s also boilerplate code. Such a design may be less convenient for the user of this class. Hence, it is classic trade-off situation, and you have to decide which of the alternatives fit better to your case.

Also, if I want to use Dependency Injection with the class and use it with IoC, then I dont think I can pass for example a “filePath” argument to the constructor, …

Reading between the lines, I feel there’s already a general agreement on your end that it would be better to tie the open/closed nature of the stream to the lifecycle of the objects, i.e. the constructor opens it, the destructor/dispose closes it. I assume this is why you immediately jump to pointing out the issues with opening the stream via the constructor.

Based on my assumption, I’m not going to further answer why because it seems you’re already in agreement on that point, the remaining question is how to achieve it.

…, since normally the dependencies are resolved by the framework/IoC lib itself.

The core of your question here seems to be about how to marry the ideas of DI-related constructor parameters and the kinds of parameters that are scoped to a particular request (e.g. a specific filename) and that cannot have its value hardcoded in the same way that DI dependencies (or their types) are.

For the sake of brevity I’m going to refer to these as “dependencies” (DI) and “parameters” (e.g. the filename) in the rest of the answer.

The question I’m going to answer here is how a constructor can accept both dependencies and parameters at the same time. This should resolve the root problem that led you to instead trying to design these Open/Close methods.

When an object receives dependencies, the constructor is being called by the DI framework itself. This mean that you are only able to provide constructor arguments that are known by the DI framework, which in turn limits you to knowledge that you had at the time of configuring the DI framework. This seems to make it impossible for such a constructor to contain a parameter.

That’s true. By the time I show you my answer, you’ll see that the object in question will no longer be constructed by the DI framework, specifically to work around this problem.

But then we would face the opposite problem. Because we are now constructing this object outside of the DI container, we lose access to its knowledge about which dependencies need to be provided to the constructor. How do we retain the knowledge of the DI framework without being tied to the DI framework calling the constructor itself?

This can be achieved by introducing an intermediary factory layer. The factory produces the object. This means that we can provide the parameter values from calling the factory method. But at the same time, the factory itself can be registered in (and generated from) the DI container, which means that it can have all the dependencies you need injected into it (the factory itself), which it can then pass on to the object that it creates.

In the following example, I’ve made it so your CsvWriter takes in a dependency (ILogger) and a parameter (filename):

public class CsvWriter : IDisposable
{
    private readonly ILogger logger;
    private readonly string fileName;

    private readonly CsvReader csvReader;

    public CsvWriter(Ilogger logger, string fileName)
    {
        this.logger = logger;
        this.fileName = fileName;

        logger.Log($"Opening file {filename}");
        this.csvReader = GetCsvReader(fileName);
    }

    public void Dispose()
    {
        csvReader.Dispose();
    }
}

I’ve omitted the content of the private GetCsvReader method for brevity, it’s essentially what you did in your Open method.

I hope you agree that this is what your ultimate goal was, i.e. to have a clean and simple CsvWriter implementation. But now we’ve also set the stage for the problem, how do we solve it? Well, the intermediary factory would look like this:

public class CsvWriterFactory
{
    private readonly ILogger logger;

    public CsvWriterFactory(ILogger logger)
    {
        this.logger = logger;
    }

    public CsvWriter Create(string fileName)
    {
        return new CsvWriter(this.logger, fileName);
    }
}

Look at the CsvWriter constructor and the two arguments we’re passing in. Now trace the origin of these arguments.

• Assuming this factory was instantiated by the DI container, the DI container will have provided the ILogger dependency; thus using the knowledge of the DI container.
• The fileName is given by whoever calls this method. The filename was not known in advance in the same way that the logger was.

Now let’s look at how you would use this. If you have a service that want to be able to make use of a CsvWriter, it can simply have a CsvWriterFactory injected. For example:

public class PersonService
{
    private reasonly CsvWriterFactory factory;

    public PersonService(CsvWriterFactory factory)
    {
        this.factory = factory;
    }

    public void Save(Person person)
    {
        string filename = $"{person.FirstName}-{person.LastName}.csv";

        using(var writer = factory.Create(filename))
        {
            writer.WriteCell(person.FirstName);
            writer.WriteCell(person.LastName);
            writer.WriteCell(person.Age);
            writer.WriteCell(person.Address);
        }
    }
}

This is, in my opinion, the cleanest way to implement all of this. Notice how the consumer (PersonService) did not need to care about the writer’s dependency (ILogger) in any way, and at the same time CsvWriter was able to tie its lifecycle to that of the underlying disposable components without needing to resort to clumsy Open/Close logic.

This is the route I’d take if I were you.

I would avoid reusing the same instance for different purposes, if the objects have any kind of identity. Say you have instances representing customers. And among other things, a view displaying customers holds references to those instances.

If one of those instances are reused, then that view will display the wrong data.