Skip to main content
4 of 5
replaced http://programmers.stackexchange.com/ with https://softwareengineering.stackexchange.com/

Some quick shots

  • ValidateBufferArgs() is not needed, because the Buffer.BlockCopy() method will handle these cases in the same manner. In addition, why do you call the ValidtaeBufferArgs() method before you check for if (!bIsOpen_m) inside the Write() method ?

  • the naming of your member variables is well strange. First you are using hungarian notation which is an anti pattern if it is used this way. There are ways how this could be acceptable but not like this

private bool bIsOpen_m; indicating by the b prefix that it is a bool.

Next, you postfix the member variables with a _m and you always use this. with them. If you pre- or postfix the variables with either m_ or _m than there is no need to use this..

EDIT:
if you e.g prefix an int variable like

    int iLength = 1;  

and you later decide that a long would be the better fit, you for sure will change the type, but the possibility exists (at a high level) that you miss to change the variables name too and you would have

    long iLength =1;  

in your code which is just lying.

Please read also: what-is-the-benefit-of-not-using-hungarian-notation

From the first answer

Because its orginal intention (see http://www.joelonsoftware.com/articles/Wrong.html and http://fplanque.net/Blog/devblog/2005/05/11/hungarian_notation_on_steroids) has been misunderstood and it has been (ab)used to help people remember what type a variable is when the language they use is not statically typed. In any statically typed language you do not need the added ballast of prefixes to tell you what type a variable is. In many untyped script languages it can help, but it has often been abused to the point of becoming totally unwieldy. Unfortunately, instead of going back to the original intent of Hungarian notation, people have just made it into one of those "evil" things you should avoid.

From the second answer

It uselessly repeats information with no benefit and additional maintenance overhead. What happens when you change your int to a different type like long, now you have to search and replace your entire code base to rename all the variables or they are now semantically wrong which is worse than if you hadn't duplicated the type in the name.

A fine read about hungarian notation (how it can be done right) is in the first answer: http://www.joelonsoftware.com/articles/Wrong.html

  • you have many places where it is obvious what type you are assigning by checking the right side of the assignment. In such cases you should use the var type and let the compiler determine the correct type.
byte[] newBuffer = new byte[count];  

would e.g become

    var newBuffer = new byte[count];
  • IDisposable

Please read: proper-use-of-the-idisposable-interface

If the user calls Dispose() on your object, then everything has been cleaned up. Later on, when the garbage collector comes along and calls Finalize, it will then call Dispose again.

Not only is this wasteful, but if your object has junk references to objects you already disposed of from the last call to Dispose(), you'll try to dispose them again!

  • Regions

Please read are-regions-an-antipattern-or-code-smell

Is there a good use for regions?

No. There was a legacy use: generated code. Still, code generation tools just have to use partial classes instead. If C# has regions support, it's mostly because this legacy use, and because now that too many people used regions in their code, it would be impossible to remove them without breaking existent codebases.

Think about it as about goto. The fact that the language or the IDE supports a feature doesn't mean that it should be used daily. StyleCop SA1124 rule is clear: you should not use regions. Never.

Heslacher
  • 51k
  • 5
  • 83
  • 177