tags:

views:

235

answers:

3

I have a Location class which represents a location somewhere in a stream. (The class isn't coupled to any specific stream.) The location information will be used to match tokens to location in the input in my parser, to allow for nicer error reporting to the user.

I want to add location tracking to a TextReader instance. This way, while reading tokens, I can grab the location (which is updated by the TextReader as data is read) and give it to the token during the tokenization process.

I am looking for a good approach on accomplishing this goal. I have come up with several designs.

Manual location tracking

Every time I need to read from the TextReader, I call AdvanceString on the Location object of the tokenizer with the data read.

Advantages

  • Very simple.
  • No class bloat.
  • No need to rewrite the TextReader methods.

Disadvantages

  • Couples location tracking logic to tokenization process.
  • Easy to forget to track something (though unit testing helps with this).
  • Bloats existing code.

Plain TextReader wrapper

Create a LocatedTextReaderWrapper class which surrounds each method call, tracking a Location property. Example:

public class LocatedTextReaderWrapper : TextReader {
    private TextReader source;

    public Location Location {
        get;
        set;
    }

    public LocatedTextReaderWrapper(TextReader source) :
        this(source, new Location()) {
    }

    public LocatedTextReaderWrapper(TextReader source, Location location) {
        this.Location = location;
        this.source = source;
    }

    public override int Read(char[] buffer, int index, int count) {
        int ret = this.source.Read(buffer, index, count);

        if(ret >= 0) {
            this.location.AdvanceString(string.Concat(buffer.Skip(index).Take(count)));
        }

        return ret;
    }

    // etc.
}

Advantages

  • Tokenization doesn't know about Location tracking.

Disadvantages

  • User needs to create and dispose a LocatedTextReaderWrapper instance, in addition to their TextReader instance.
  • Doesn't allow different types of tracking or different location trackers to be added without layers of wrappers.

Event-based TextReader wrapper

Like LocatedTextReaderWrapper, but decouples it from the Location object raising an event whenever data is read.

Advantages

  • Can be reused for other types of tracking.
  • Tokenization doesn't know about Location tracking or other tracking.
  • Can have multiple, independent Location objects (or other methods of tracking) tracking at once.

Disadvantages

  • Requires boilerplate code to enable location tracking.
  • User needs to create and dispose the wrapper instance, in addition to their TextReader instance.

Aspect-orientated approach

Use AOP to perform like the event-based wrapper approach.

Advantages

  • Can be reused for other types of tracking.
  • Tokenization doesn't know about Location tracking or other tracking.
  • No need to rewrite the TextReader methods.

Disadvantages

  • Requires external dependencies, which I want to avoid.

I am looking for the best approach in my situation. I would like to:

  • Not bloat the tokenizer methods with location tracking.
  • Not require heavy initialization in user code.
  • Not have any/much boilerplate/duplicated code.
  • (Perhaps) not couple the TextReader with the Location class.

Any insight into this problem and possible solutions or adjustments are welcome. Thanks!

(For those who want a specific question: What is the best way to wrap the functionality of a TextReader?)

I have implemented the "Plain TextReader wrapper" and "Event-based TextReader wrapper" approaches and am displeased with both, for reasons mentioned in their disadvantages.

+1  A: 

I would go for the plain TextReader wrapper, because it seems the most natural approach. Also, I don't think the disadvantages you mentioned are really disadvantages :

  • User needs to create and dispose a LocatedTextReaderWrapper instance, in addition to their TextReader instance.

Well, if the user needs tracking, he will need to manipulate tracking-related objects anyway, and creating a wrapper isn't such a big deal... To make things easier, you could create an extension method that creates a tracking wrapper for any TextReader.

  • Doesn't allow different types of tracking or different location trackers to be added without layers of wrappers.

You could always use a collection of Location objects, rather than a single one :

public class LocatedTextReaderWrapper : TextReader {

    private TextReader source;

    public IList<Location> Locations {
        get;
        private set;
    }

    public LocatedTextReaderWrapper(TextReader source, params Location[] locations) {
        this.Locations = new List<Location>(locations);
        this.source = source;
    }

    public override int Read(char[] buffer, int index, int count) {
        int ret = this.source.Read(buffer, index, count);

        if(ret >= 0) {
            var s = string.Concat(buffer.Skip(index).Take(count));
            foreach(Location loc in this.Locations)
            {
                loc.AdvanceString(s);
            }
        }

        return ret;
    }

    // etc.
}
Thomas Levesque
Thanks for your input; your solution seems like a good one. Still, I don't like seeing `using(var reader = new LocatedTextReaderWrapper(input)) {` all over my unit tests which don't care about location tracking. Then again, I'd need some weird code otherwise. An extension method seems like a good idea; I'll try that.
strager
+4  A: 

I agree that creating a wrapper that implements TextReader, delegates the implementation to the underlying TextReader and adds some additional support for tracking of location is a good approach. You can think of this as the Decorator pattern, because you're decorating the TextReader class with some additional functionality.

Better wrapping: You can write it in a simpler way to decouple your TextReader from the Location type - this way you can easily modify Location independently or even provide other features that are based on tracking the progress of reading.

interface ITracker {
  void AdvanceString(string s);
}

class TrackingReader : TextReader {
  private TextReader source;
  private ITracker tracker;
  public TrackingReader(TextReader source, ITracker tracker) {
    this.source = source;
    this.tracker = tracker;
  }
  public override int Read(char[] buffer, int index, int count) {
    int res = base.Read(buffer, index, count);
    tracker.AdvanceString(buffer.Skip(index).Take(res);
    return res;
  }
}

I would also encapsulate creation of the tracker into some factory method (so that you have a single place in the application that deals with construction). Note that you can use this simple desing to create a TextReader that reports the progress to multiple Location objects too:

static TextReader CreateReader(TextReader source, params ITracker[] trackers) {
   return trackers.Aggregate(source, (reader, tracker) =>
       new TrackingReader(reader, tracker));
}

This creates a chain of TrackingReader objects and each of the object is reporting the progress of reading to one of the trackers passed as arguments.

Regarding Event-based: I think that using standard .NET/C# events for this kind of thing isn't done as frequently in the .NET libraries, but I think this approach may be quite interesting too - especially in C# 3 where you can use features like lambda expressions or even Reactive Framework.

Simple use doesn't add that much boiler plate:

using(ReaderWithEvents reader = new ReaderWithEvents(source)) {
  reader.Advance += str => loc.AdvanceString(str);
  // ..
}

However, you could also use Reactive Extensions to process the events. To count the number of new lines in the source text, you could write something like this:

var res =
  (from e in Observable.FromEvent(reader, "Advance")
   select e.Count(ch => ch == '\n')).Scan(0, (sum, current) => sum + current);

This would give you an event res that fires each time you read some string from the TextReader and the value carried by the event would be the current number of line (in the text).

Tomas Petricek
For "better wrapping", that's basically what I had in mind when looking at @Thomas Levesque's answer. Your event-based idea looks better than what I originally had in mind, and I'll look into that more. Using Reactive Extensions doesn't seem like a good alternative, though, as it's an extra dependency (it looks like?) and produces odd-looking code (IMO).
strager
@strager: Reactive Extensions are extra dependency, but it's actually easy to re-implement them at a very basic level (http://tomasp.net/blog/reactive-ii-csevents.aspx). The code looks a bit unfamiliar - and I didn't really explain it in my answer - however, if you become more familiar with it, it is quite powerful... (but it may not be the best choice for your application).
Tomas Petricek
A: 

AFAIK, if you use PostSharp, it injects itself into the compiling process and rewrites code as necessary so you don't have any eternal dependencies to ship (other than requiring API users to have PostSharp to compile your source).

RCIX
Sadly, even this isn't an option for me. It sounds like a good alternative to larger AOP libraries, though.
strager
@strager: if you don't mind, can you elaborate on why?
RCIX