Return to Answer

• 1

• 1

• 50.9k
• 5
• 83
• 177

Constructors

Because the constructor of the StreamReader [(Stream, Encoding)][1] is equal to calling the overloaded constructor [(Stream, Encoding, bool)][2] with true for the bool parameter, you should use constructor chaining.

From the first link:

The StreamReader object attempts to detect the encoding by looking at the first three bytes of the stream. It automatically recognizes UTF-8, little-endian Unicode, and big-endian Unicode text if the file starts with the appropriate byte order marks. Otherwise, the user-provided encoding is used.

So you could simplify your code like

public EncodingTranslatorStream(System.IO.Stream strInput, Encoding encodingIn, Encoding encodingOut) 
 : this(strInput, encodingIn, true, encodingOut)
{}

Dead code like //this.strOut_m = new MemoryStream(); should be removed because it only adds noise to the code instead of any value.

The same is true for comments which are saying what is done instead of why something is done. Please read this fine answer about comments: [http://codereview.stackexchange.com/a/90113/29371][3]

Possible problems

The overridden Position property can cause problems if a not correctly coded subclass of stream is passed to the constructor.

Usually a Stream should throw a NotSupportedException if the Position property is set and the stream is not seekable. So if you check CanSeek before setting the Position you have done everything you can do to prevent any exceptions. If a subclass of the Stream class is passed in, which doesn't follow this pattern, noone can blame you then.

Again the naming

This block of code

if (this.srInput_m.CurrentEncoding.Equals(this.swOutput_m.Encoding))
{
 //The encodings are the same, lets just bypass the translation stuff
 return this.strInput_m.Read(buffer, offset, count);
}

is very hard to read/grasp at first glance. The difference between srInput_m and strInput_m is extremely small.

You should at least change sr_Input_m to reader or streamReader with or without your postfix of choice where as I would not use it if you always use this.

Validation

In the Read() method you have

//Validate the parameters passed in
this.ValidateBufferArgs(buffer, offset, count); 

but you are using the parameters before the check if the encoding of the used StreamReader equals the encoding of the used StreamWriter.

Either omit the check (I guess the stream and streamreader take care of this) or move it to the top of the method.

Read() method

Basically the Read() method is way to long. You should consider to break it into smaller methods which are easier to maintain and also better to read. [1]: https://msdn.microsoft.com/en-us/library/x8xxf0x5%28v=vs.110%29.aspx [2]: https://msdn.microsoft.com/en-us/library/ms143457%28v=vs.110%29.aspx [3]: http://codereview.stackexchange.com/a/90113/29371