CODEWITHSUNDEEP
c#delegatesanonymous-functionanonymous-methods
Scott Smith
Scott Smith

Reputation: 3986

Use an anonymous method to avoid creating a single-use object?

I'm trying to refactor a method that parses through a file. To support files of arbitrary size, the method using a chunking approach with a fixed buffer.

public int Parse()
{
    // Get the initial chunk of data
    ReadNextChunk();

    while (lengthOfDataInBuffer > 0)
    {
       [parse through contents of buffer]

       if (buffer_is_about_to_underflow)
          ReadNextChunk();
    }

    return result;
}

The pseudo code above is part of the only public non-static method in a class (other than the constructor). The class only exists to encapsulate the state that must be tracked while parsing through a file. Further, once this method has been called on the class, it can't/shouldn't be called again. So the usage pattern looks like this:

var obj = new MyClass(filenameToParse);

var result = obj.Parse();

// Never use 'obj' instance again after this.

This bugs me for some reason. I could make the MyClass constructor private, change Parse to a static method, and have the Parse method new up an instance of Parse scoped to the method. That would yield a usage pattern like the following:

var result = MyClass.Parse(filenameToParse);

MyClass isn't a static class though; I still have to create a local instance in the Parse method.

Since this class only has two methods; Parse and (private) ReadNextChunk, I'm wondering if it might not be cleaner to write Parse as a single static method by embedding the ReadNextChunk logic within Parse as an anonymous method. The rest of the state could be tracked as local variables instead of member variables.

Of course, I could accomplish something similar by making ReadNextChunk a static method, and then passing all of the context in, but I remember that anon methods had access to the outer scope.

Is this weird and ugly, or a reasonable approach?

Upvotes: 2

Views: 272

Answers (3)

nathanchere
nathanchere

Reputation: 8098

MyClass isn't a static class though; I still have to create a local instance in the Parse method.

You don't need a static class to be able to leverage static methods. For example this works fine:

public class MyClass
{
    public static string DoStuff(string input)
    {
        Console.WriteLine("Did stuff: " + input);
        return "Did stuff";
    }
}

public class Host
{
    public void Main()
    {
        MyClass.DoStuff("something");
    }
}

Upvotes: 0

Keith Nicholas
Keith Nicholas

Reputation: 44298

All you need is a static parse method that creates an instance, much like what you suggest in your question

public class MyClass
{
     // your existing code.... but make the members and constructor private.


     public static int Parse(string filenameToParse)
     {
         return new MyClass(filenameToParse).Parse();
     }
}

then

just use it like you suggest...

var result = MyClass.Parse(filenameToParse);

Upvotes: 0

Fendy
Fendy

Reputation: 4643

This maybe suitable more to code review.

However, these are my comments about your design:

  1. I don't think it will matter much about obj instance only used once. If you bugged with it, there are 2 ways to trick it:

    • Use of another method such as:

      public int Parse()
{
    var obj = new MyClass(filenameToParse);
    return obj.Parse();
}

    • Make the MyClass implement IDisposable and wrap it in using statement. I don't recommend this since usually IDisposable has logic in their Dispose() method

  2. I think it is better to make your MyClass accept parameter in Parse to Parse(string fileNameToParse). It will make MyClass as a service class, make it stateless, reusable and injectable.

  3. Regarding impact to static class. First it add coupling between your consumer and MyClass. Sometimes if you want to test / unit test the consumer without using the MyClass parser, it will be hard / impossible to mock the MyClass into something you want.

Upvotes: 4

Related Questions