7

I have a cyclic dependency in my architecture design.

Let's assume there are the following modules:

FileSystem (uses Logger)

Handles file operations like reading, writing files etc. This module should log its actions. The logger is injected via dependency injection.


NullLogger (implements Logger)

Discards all log messages.


FileLogger (implements Logger)

Writes log messages to the file system. Should internally use the FileSystem module to reduce code duplication. (See where its going?)


ConsoleLogger (implements Logger)

Writes log messages directly to the console.


The problem (if its not clear by now):

  • I pass ConsoleLogger to FileSystem and everything works fine.
  • I pass FileLogger to FileSystem, and poof! It won't work because those two modules are recursively dependent on each other so I won't be able to instantiate either of them.

I can only think of one possible way to avoid the problem:

FileLogger instantiates it's own FileSystem with a NullLogger (or any other Logger except FileLogger). Drawback: I would not be able to see any log messages (in the file) from the FileLogger itself.

But I'm not sure if that's the route I should go for.

  • 10
    If your logger infrastructure also logs its own actions, then you will get such an explosion of log messages that the noise from the logging infrastructure will completely drown the useful information from the rest of the application. – Bart van Ingen Schenau Apr 14 '17 at 6:25
  • an Interface described by a new component which the other two subscribe to would alleviate your dependency problem – JonathanC Apr 15 '17 at 9:07
  • This would be much simpler if you stick to your application concerns. The app shouldn't care where logs go, it doesn't matter. If you always log go standard out and standard error, your environment can decide to redirect those streams to a file or database or web service or anywhere else. Check out 12factor.net – Paul Apr 17 '17 at 0:59
17

I pass FileLogger to FileSystem, and poof! It won't work because those two modules are recursively dependent on each other so I won't be able to instantiate either of them.

I think it is debatable if a FileSystem module really needs logging at its core, or if this concern should be solved on a different level, like suggested by Ben Cotrell, but anyway, let us assume your requirement is justified for some reason.

Logs of FileSystem calls should be written to a different file than the log file used by, for example, an application which instantiates a file logger for a different purpose. So there should be two FileLogger objects in this scenario, each one associated with a different file. And since each of your FileSystem objects holds a reference to a specific file logger, this leads to the need of having two different file system objects as well, where the first one is initialized with a null logger and the second one with the logger for file system calls. In pseudo code:

 FileSystem nonLoggingFileSystem = new FileSystem(new NullLogger());

 FileLogger fileSysCallsLogger("file_system_calls.log", nonLoggingFileSystem);

 FileSystem fileSystem = new FileSystem(fileSysCallsLogger);

 FileLogger applicationCallsLogger(`application_calls.log`, fileSystem);

Now there is no cyclic dependency between the involved objects, and fileSystem as well as applicationCallsLogger can be used safely inside the application, without any recursion.

  • Yes, this is what I was going for :) – d3L Apr 14 '17 at 11:49
  • Interesting 1st sentence. Reminds me of admonitions against (unit) testing a framework. – radarbob Apr 24 '17 at 21:25
  • @radarbob: don't get me wrong, logging in combination with file system operations can make a lot sense. However, I think Ben Cottrell's approach to create a file system component without any logging code first and then use decorating techniques to create a logging file system on top is not bad and also a valid solution, that is why I wrote "it is debatable". – Doc Brown Apr 25 '17 at 5:34
3

Update to clarify the intent of this answer: The question identifies two separate responsibilities as per Single Responsibility Principle - logging and file I/O. One of the options presented in the question is to inject a NullLogger which does nothing. An alternative to injecting a do-nothing NullLogger could be to write a standalone class to handle the boundary which has no interest in logging, then handle the logging higher up, and eliminating the need for a NullLogger.


Consider splitting the behaviour of your FileSystem between the code which actually performs the I/O and that code which decides if/when to perform any related logging (e.g. LoggedFileSystem).

You could have a simple FileSystem class which performs no logging, and is only used by the Logger, and by the LoggedFileSystem, while the rest of the system uses LoggedFileSystem for all of its File I/O.

For example:

// The "raw" I/O class.
public class FileSystem 
{
    public byte[] Read(string filename) { /* ..etc. */ }
    public void Write(byte[] data, string filename) { /* ..etc. */ }
}

// Used by the rest of the system for its I/O operations
public class LoggedFileSystem
{
    private FileSystem _fileSystem = new FileSystem();

    public byte[] Read(string filename) 
    { 
        try 
        {
            Logger.Debug($"Reading file {filename}...");
            var data = _fileSystem.Read(fileName);
            Logger.Debug($"Successfully read {filename} - {data.Length} bytes");
            return data;
        }
        catch (Exception ex)
        {
            Logger.Error($"Exception handled reading {filename}", ex);
            throw;
        }
    }

    public void Write(byte[] data, string filename) 
    { 
        try 
        {
            Logger.Debug($"Writing {data.Length} bytes to {filename}");
            var data = _fileSystem.Read(fileName);
            Logger.Debug($"FileSystem.Write succeeded.");
        }
        catch (Exception ex)
        {
            Logger.Error($"Exception handled writing to {filename}", ex);
            throw;
        }
    }
}

In summary, this alternative suggestion is functionally equivalent to one of the original suggestions of injecting a NullLogger into aFileSystem`. The result is

  • A FileSystem class which has no logging,
  • A LoggedFileSystem class with the same methods as FileSystem (to be used in the same way from an outsider perspective), but which also handles logging.

Update: As a side note: you might still inject the Logger into the LoggedFileSystem as per some of the other alternative solutions, in case that's needed for any reason:

public class LoggedFileSystem
{
    private FileSystem _fileSystem = new FileSystem();
    private ILogger _logger;

    public LoggedFileSystem(ILogger logger)
    {
        _logger = logger;
    }
} 
  • What would be the benefit of this solution over just passing a NullLogger to FileSystem for the FileLogger? – d3L Apr 14 '17 at 11:41
  • @d3l They're functionally equivalent, but this approach is about keeping with the Single Responsibility Principle (SRP). A class representing File I/O is an external boundary, so ideally its responsibility would be limited just to I/O concerns. If the class also decides when and what to log that starts to look like a violation of SRP. Logic for Logging is one of those cross-cutting concerns, similar to Caching; this approach is about catering for cross-cutting concerns without touching the actual 'boundary' (I/O) logic. – Ben Cottrell Apr 14 '17 at 12:01
  • I disagree with your conception of "responsibility" in this case. Responsibility is a subjective concept and should change according to the abstraction level. – Frank Hileman Apr 15 '17 at 20:04
  • @FrankHileman To clarify, I'm not basing it on subjective opinion; I linked to the wikipedia page for Single Responsibility Principle, so please assume I am referring to that definition. The question asker has identified two separate responsibilities - logging and file I/O, so the fact that these are indeed separate responsibilities seems to be a "given" (being that it's the entire basis for the question); this answer is an alternative way of splitting those responsibilities by isolating the boundary entirely. – Ben Cottrell Apr 15 '17 at 22:17
  • @FrankHileman Added extra context information at the top of the answer with the hope it clarifies 'responsibility' in more objective terms. – Ben Cottrell Apr 16 '17 at 9:02
1

As long as your objects don't do any useful work in their constructors, it is possible to resolve cyclic dependencies in dependency injection by using property injection.

For example, if your FileLogger defers opening its file until first called to log something, you could initialise your system like this:

FileLogger logger = new FileLogger();
FileSystem filesystem = new FileSystem (logger);
logger.setFileSystem (filesystem);

You could also choose to break the cycle the other way, by injecting the file system's logger as a property.

It would be important for FileLogger to have a good way of recovering if used before the file system property is set (perhaps storing messages in a buffer and logging them at the first opportunity).

  • I thought of that but this solution requires the logger to be aware of the FileSystem, which I want to avoid at all cost. – d3L Apr 14 '17 at 11:50
  • 1
    Then break it the other way. Initialise FileSystem with a MemoryLogger, then when the FileLogger is initialized, change ît to the FileLogger and dump anything in the MemoryLogger's buffer into the FileLogger. – Jules Apr 14 '17 at 16:13

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.