Idiomatic C# for object only interacting though having registered to events

Question

I am uneasy about the way I have designed a simple program. There is a FileParser object which has OnFileOpened, OnLineParsed, and OnFileClosed events, and several objects which create smaller files based on the content of the file which the FileParser parses.

The objects observing the FileParser register their methods with the FileParser events in their constructors.

ParsedFileFrobber(FileParser fileParser)
{
    fileParser.OnFileOpen += this.OpenNewFrobFile;
    fileParser.OnLineParsed += this.WriteFrobbedLine;
    fileParser.OnFileClose += this.CloseFrobFile;
}

After that, the ParsedFileFrobber just gets on with things without further explicit interaction.

The thing that bothers me is that the main program only ever assigns a ParsedFileFrobber and basically doesn't use the return value of the constructor, so to say.

var fileParser = new FileParser(myFilename);
var parsedFileFrobber = new ParsedFileFrobber(fileParser);
// No further mentions of parsedFileFrobber.

It works, but ReSharper complains about this, which at least makes me pause to think. Actually, I don't even need to assign a variable to the result of the constructor, as the GC will keep the ParsedFileFrobber alive by merit of the event handler references, but a bare new looks very wrong to me. (It still compiles and runs correctly.)

var fileParser = new FileParser(myFilename);
new ParsedFileFrobber(fileParser);

Is this a problem? Is it an anti-pattern or a code smell? Is there already an idiomatic way of doing this in C#?

Thanks!

Clarifications thanks to helpful comments:

1) Why not invert the relationship? Cody Gray

Ah, the example was a bit too simplified. I actually have a ParsedFileFrobber, a ParsedFileGrobber, and a ParsedFileBrobber. Inverting the relationship would make the FileParser dependent on all 3. (Also, initially there was only a Frobber and a Grobber, but later on there was a need for a Brobber, and there is still scope for uh, a Drobber and so forth.) I guess that it's a matter of taste about whether the lines of code doing the subscription happen in the FileParser constructor or in the ParsedFileFrobber, ParsedFileGrobber, and ParsedFileBrobber constructors, but my preference is to try to keep the FileParser as agnostic as possible.

2) Why not move the constructor into a static method (and make the constructor private)? Hans Passant

I can see how that tidies away the possibly-unintuitive usage into the private inner workings of the class, which is good advice. However, it would still be either the bare new or the only-ever-assigned reference to the return value of the constructor. Well, if it's not a big problem, it makes sense to hide away the ugly code. (For reference, I did make the ParsedFileFrobber an IDisposable with the Dispose method unsubscribing from the events, so it is possible to put an end to the frobbing.)

Thank you to all commenters!

Answer 1

One possible solution could be to use the Mediator pattern, which introduces a new class who controls the interaction between your FileParser and the ParsedFile* classes:. That way your classes are very loosely coupled, your FileParser does not need to know about the ParsedFileFrobber and vice versa. The mediator class can also take care of the lifetime of the objects.

public interface IParsedFile
{
    void OnOpen();
    void OnLineParsed(string line);
    void OnClosed();
}

public class FileParseManager
{
    private readonly FileParser _fileParser;
    private readonly List<IParsedFile> _parsedFiles;

    public FileParseManager(FileParser fileParser, List<IParsedFile> parsedFiles)
    {
        _fileParser = fileParser;
        _parsedFiles = parsedFiles;
    }

    public void Parse(string fileName)
    {
        _fileParser.OpenFile(string fileName);
        foreach (var parsedFile in _parsedFiles)
        {
            parsedFile.OnOpen();
        }

        while ((string line = _fileParser.GetNextLine()) != null)
        {
            foreach (var parsedFile in _parsedFiles)
            {
                parsedFile.OnLineParsed(line);
            }
        }

        _fileParser.CloseFile();
        foreach (var parsedFile in _parsedFiles)
        {
            parsedFile.OnClosed();
        }
    }
}

Another option might be to use a producer-consumer pattern. Something like this is provided e.g. by the TPL (Task Parallel Library) in the .NET Framework. Have a look at the Dataflow section and the examples provided there.