1

I have an class named FileCreator which is used to write many strings to a stream. Basically, to achieve its job, the FileCreator needs two objects: a StreamWriter and the actual strings that will be written to the stream. So far so good.

Now, on a architectural level, I'm wondering what would be the proper way to pass the Streamwriter to the FileCreator. I see three solutions, but all have their downsides:

Solution 1:

In the FileCreator class, create a method named let's say Write that would receive the StreamWriter and the string to write. For example, in c# it would be:

public void Write(StreamWriter sw, string stringToWrite);

Issue with this solution: This solution would work, but the StreamWriter needs to be passed every time a string needs to be written. It feels weird, and not OO at all.

Solution 2:

The FileCreate should have 2 methods:

public void SetWriter(StreamWriter writer);
public void Write(string stringToWrite);

This solution would work, but it brings temporal coupling. Basically, the caller must know that SetWriter must be called one before the calls to Write. And let's say the Write method is called before setting the Writer... throw a custom exception indicating something like 'Writer should be set'? ... feels weird.

Solution 3:

Make the FileCreator constructor have the StreamWriter as a parameter to prevent the issue raised in the previous solution.

For example:

public FileCreator (StreamWriter writer);
public void Write(string stringToWrite);

While this may work well in some situations, in our system, it would not. This is because we are doing unit tests with dependancy injection. In our situation, the StreamWriter cannot be injected because some of its properties like EOL characters, Encoding and FilePath are determined by business rules at run time in other methods.

So in your opinion, what would be the appropriate solution? Maybe there are other techniques that could be applied? Hope my question in clear!

  • 3
    2 I would definitely avoid; 1 is acceptable but annoying; 3 is what I would go for since it seems like FileCreator is a decorator. I have my doubts that it's really impossible to inject what you need, can you give more detail and why you think option 3 wont work for you? – xtratic Nov 19 '19 at 15:39
  • 1
    "because some of its properties like EOL characters, Encoding and FilePath are determined by business rules at run time in other methods" - so? Could you explain in more detail the exact reason that makes this a problem; alternatively, maybe you can redesign so that you don't rely on globals as much. – Filip Milovanović Nov 19 '19 at 16:49
  • Is it actually a problem, because the file path, for example, used in the StreamWriter constructor, is not known when the application is started up. That path is specified by the end user in the UI. Because of that, the FileCreator cannot be injected. – user1861857 Nov 19 '19 at 19:06
  • Option 3 would not work because the StreamWriter requires a filepath which is unknown at the startup. Its value is determined at run time based on users inputs. – user1861857 Nov 19 '19 at 19:14
3

Passing the StreamWriter as a constructor argument to FileCreator is the preferred approach if the stream writer is indeed a shared resource between all method calls against any one particular FileCreator object.

If you need to specify different stream writers for different method invocations on FileCreator, then you need to pass the StreamWriter as an argument to a method. I would avoid this if at all possible, since it will make testing dependent types and methods more difficult.

Because FileCreator ends up doing I/O, testing the FileCreator class feels like functional testing is more appropriate. You would need a real StreamWriter that writes to a file or memory.

The problems comes in when testing classes that depend on FileCreator (as opposed to testing FileCreator itself), and this is where your current architecture breaks down. Trying to mock the StreamWriter or Stream is not the correct approach. You need to mock the FileCreator itself.

Create an interface for FileCreator, and then have it implement that interface:

public interface IFileCreator
{
    void Write(string stringToWrite);
}

public class FileCreator : IFileCreator
{
    private readonly StreamWriter writer;

    public FileCreator(StreamWriter writer)
    {
        this.writer = writer;
    }

    public void Write(string stringToWrite)
    {
        // ...
    }
}

Now other classes that need the FileCreator can use it through the interface:

public void SomeMethodThatNeedsIt(IFileCreator fileCreator)
{
    fileCreator.Write("...");
}

public class SomeTypeThatNeedsIt
{
    private readonly IFileCreator fileCreator;

    public SomeTypeThatNeedsIt(IFileCreator fileCreator)
    {
        this.fileCreator = fileCreator;
    }
}

Unit testing becomes much easier now, because you can mock a discrete, limited interface.

So the answer is:

  1. Write functional tests for FileCreator using a real StreamWriter and MemoryStream
  2. Create an interface for FileCreator and have all dependent types and methods use a FileCreator through the IFileCreator interface
  3. Mock the IFileCreator objects when unit testing the dependent types and methods.

To address your concerns about passing StreamWriter as a constructor argument:

Make the FileCreator constructor have the StreamWriter as a parameter to prevent the issue raised in the previous solution.

While this may work well in some situations, in our system, it would not. This is because we are doing unit tests with dependancy injection. In our situation, the StreamWriter cannot be injected because some of its properties like EOL characters, Encoding and FilePath are determined by business rules at run time in other methods.

If some values are determined at run time, then a centralized dependency injection strategy (like you find with dependency injection frameworks) is not appropriate for the FileCreator class. It should be initialized where you know all the information it depends on, and that is not likely known during the initialization of the application where most DI frameworks are configured.

The types that depend on the file creator should be initialized at the point where you can determine the StreamWriter, or delegate initialization of the FileCreator to a factory method or object. The factory object could be registered with a dependency injection container, which returns IFileCreator objects.

| improve this answer | |
  • So what you are suggesting is that, since the filecreator cannot be injected, instead a factory object could be injected. For example, FileCreatorFactory. This factory could expose a method named GetCreator(StreamWriter writer) which would simply pass the writer in the constructor of the FileCreator, and return a IFileCreator? – user1861857 Nov 19 '19 at 19:10
  • @user1861857: Yes, precisely. The file creator factory is an additional level of indirection, but it allows for DI and polymorphism to play nicely with the need for runtime data. – Greg Burghardt Nov 19 '19 at 19:16
0

Since the FileCreator class requires the StreamWriter in order to work correctly, that means that a FileCreator without a StreamWriter makes no sense, which in turn means that it should require the StreamWriter in its constructor. (Your [3] solution.) There are now a few problems left to solve:

  1. The Dependency Inversion Principle states that "details should depend on abstractions". The StreamWriter class is too concrete for your needs as a client (that is, from the point of view of the FileCreator class). You need an IWriter interface which in turn, since StreamWriter does not implement IWriter, means you also need a WriterAdapter class. (To be honest, it might also be considered a Facade.)

  2. Now the problem becomes that you cannot, as you stated, accept the writer at construction time because some information is unknown at that time. This problem can be solved, as indicated in another response, with a factory. At the time we need the underlying writer, we invoke the factory.Create method and then use the resulting IWriter.

  3. However, Abstract Factories are a code smell. The FileCreator class doesn't need to know about factories; that's an implementation detail. You should hide the factory behind a proxy:

    public class WriterProxy : IWriter
    {
        private readonly Lazy<IWriter> lazyWriter;
    
        public WriterProxy(IWriterFactory factory)
        {
            lazyWriter = new Lazy<IWriter>(() => factory.Create());
        }
    
        public void Write(string text) => lazyWriter.Value.Write(text);
    }
    

An unfortunately common complaint to this architecture is that it's too complicated. We now have IWriter, WriterAdapter, IWriterFactory, WriterFactory and WriterProxy, in addition to the original FileCreator and StreamWriter classes. Personally, the contest between "few complex hard-to-change" classes and "many simple easy-to-change" ones is easily won by the second, but YMMV.

| improve this answer | |

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.