Rotor FUBAR of the day: BinaryReader methods are marked virtual...
I should really quit looking at the darn Rotor code (or using ILDasm), because I'm just going to keep finding stuff like this that gets my goat. For those that don't know, there is a performance overhead with any method marked virtual and a call versus a callvirt have slight speed differences. So marking all of the methods virtual has the impact of making the BinaryReader a bit slower than it could be. However, this is a very slight speed difference, so maybe in the grander scheme of things it really doesn't matter.
What is even worse though is that you can write your own custom implementation of the BinaryReader. When doing so you can overload specific members, most notably FillBuffer. However, FillBuffer makes use of a private m_buffer field. Okay, so what happens if you just override FillBuffer?
Well, all of the other methods on the BinaryReader fail. The reason is that the underlying m_buffer that they all use isn't getting filled, and instead your custom FillBuffer routine is filling some other local (probably private) variable that you are working with. Now this is just retarded. There is no added value to being able to override the protected FillBuffer routine because the underlying data buffer isn't made available. There is no way to supplant only portions of the BinaryReader functionality and you would be forced to override and rewrite all of the methods anyway.
So how could they fix this?
1. Make FillBuffer private so that it can't be overriden.
2. Make m_buffer protected so that an overriding FillBuffer can work on the data buffer. Better yet, make it protected so that you can override other methods like ReadInt32 with your own implementation and still be allowed to take advantage of the underlying FillBuffer routine.
I have some more recommendations for this class that I may talk about another time.