|
Just out of curiosity, why shouldn't private members be documented using XML comments? The Microsoft C# compiler outputs XML documentation for private members in exactly the same way it does for public members. Furthermore, NDoc provides an option for including or excluding private member documentation when generating the final (formatted) documentation.
I personally use XML comments on private members (although I don't normally bother with private fields as their type and name usually describes their purpose sufficiently).
Cheers,
Richard.
|
|
|
|
|
Well, I guess using "at least" would make the sentence a bit clearer. On the other hand, I believe that XML comments are usually used for generating documentation for public consumption but there's no harm in using them for private members. It's just a matter of taste I guess.
A complex system that does not work is invariably found to have evolved from a simpler system that worked just fine. - Murphy's Law of Computing
|
|
|
|
|
Just a couple of points that I've picked up from various Microsoft articles on exception handling and best practices: -
- Never throw Exception, ApplicationException or SystemException. These exceptions convey no structured information about the error.
- Never catch Exception, ApplicationException or SystemException unless it is the final "catch all" of your program and you are doing so for debugging/logging/error reporting purposes. Catching these exceptions will mask a massive amount of problems, e.g. catching Exception or SystemException would catch InvalidProgramException; an exception that typically signifies a bug in the compiler!
- Only catch exceptions that you EXPECT to be thrown by the code in the try block. Catching more general exceptions may mask bugs.
- Never write an empty catch block unless you're ABSOLUTELY sure you want to turn a blind eye to the exception. Exceptions usually signify a problem that SHOULDN'T be ignored. When failure is likely use a return code or an out argument to signify failure instead (e.g. compare System.Double.TryParse with System.Double.Parse).
- Always aim to validate every argument passed to your method.
- Throw a InvalidEnumArgumentException if an enumeration argument is not a member of the enumeration type.
- Throw an ArgumentOutOfRangeException if a numerical argument is not within the range of valid values.
- Throw an ArgumentNullException if a reference-type argument is null and null is not a valid value.
- Throw an InvalidOperationException if a block of code cannot be executed because of an anomaly with the state of the current object (e.g. calling the Read method of a FileStream object that has been closed).
- Never throw an InvalidOperationException if a block of code cannot be executed because of an anomaly with the state of an external object. InvalidOperationException usually (always?) signifies a bug in the calling code and as such it is usually beneficial to the developer to allow these exceptions to propagate to the top of the program; throw a more meaningful exception instead.
- Avoid propagating low-level exceptions up to the next architectural layer. Instead, wrap them up into more general exceptions so that the implementation details of your abstraction are better hidden from the caller; i.e. don't throw exceptions that have little or no meaning to the caller. E.g. when I call File.Open on a protected file I would expect an UnauthorizedAccessException, not a Microsoft.Ntfs5.PermissionMismatchException.
- Always localise the strings thrown by your exceptions (use ResourceManager). Incidentally, I have heard many conflicting views on this one, but I'm just giving you my opinion. If localisation fails for any reason then I guess your best bet is to throw hard-coded English text (as most people do anyway), but this is debatable.
- Ultimately, never display the Message property of a .NET framework exception to the user. These are not guaranteed to be in the user's native language and usually convey little or no meaning to the user. Instead, only display error messages based on the Message property of your (localised) custom exceptions.
- When throwing custom exceptions, always assume the user may end up reading the message you supply, so no bad language!
Hope these help. Nice article.
Richard.
|
|
|
|
|
Great
<italic>Work hard and a bit of luck is the key to success. You don`t need to be genius, to be rich.
|
|
|
|
|
Nice. I will eventually post the update to this article and I will certainly add some of your guidelines.
A complex system that does not work is invariably found to have evolved from a simpler system that worked just fine. - Murphy's Law of Computing
|
|
|
|
|
Happy to be of service.
|
|
|
|
|
According to inside sources at Microsoft, implementation of ICloneable should be avoided due to the ambiguity of whether .Clone() method creates a shallow or deep copy. From what I've heard, they are thinking of eventually deprecating ICloneable.
Secondly, deriving from System.ApplicationException is no longer encouraged. A quote from Brad Abrams' MSDN blog,
Designing exception hierarchies is tricky. Well-designed exception hierarchies are wide, not very deep, and contain only those exceptions for which there is a programmatic scenario for catching. We added ApplicationException thinking it would add value by grouping exceptions declared outside of the .NET Framework, but there is no scenario for catching ApplicationException and it only adds unnecessary depth to the hierarchy.
JR - You should not define new exception classes derived from Application-
Exception; use Exception instead. In addition, you should not write code that
catches ApplicationException.
#include "witty_sig.h"
|
|
|
|
|
(I'm using .Net framework 1.0 on Win2K)
I have implemented a pop3 application in ASP.Net.. which downloads mail from a POP server and updates the DB on click of a button in the page.
Whenever i download large mails (of 5 MB or more) ...the memory consumption of the process 'aspnet_wp.exe' increases dramatically (like 300 MB, which is greater than 60% of the available RAM - 512MB) and is finally recycled by the OS... which results in the download being aborted.
Below i have given the piece of code which actually retrieves the data from the POP server. Please review it suggest me as to why it happens. The memory consumption increases when the processing enters this piece of code.
int BUFFERSIZE = 100000;
char [] buffer = new char[BUFFERSIZE];
int len;
string strb = "";
try
{
string prevcur,szTemp,prev;
szTemp = RdStrm.ReadLine();
prevcur = prev = szTemp+"\r\n";
was_pop_error(szTemp);
if(!error)
{
while(prevcur.IndexOf("\r\n.\r\n") == -1)
{
strb += prev;
len = 0;
len = RdStrm.Read(buffer,0,BUFFERSIZE);
string cur = new System.Text.StringBuilder("").Append(buffer,0,len).ToString();
prevcur = prev+cur;
prev = cur;
}
}
prevcur = prevcur.Replace("\r\n.\r\n","");
strb += prevcur;
return(strb);
}
catch(InvalidOperationException err)
{
return("Error in mail download" + err.ToString ());
}
Vikram S
|
|
|
|
|
Hi Vikram,
If you are doing it synchronously, then it is bound to be very memory hungry. I will like to suggest approach of async calling mechanism. That will be keeping your aspnet_wp.exe free from getting overloaded.
Jayant D. Kulkarni
Brainbench Certified Software Engineer in C#, ASP.NET, .NET Framework and ADO.NET
|
|
|
|
|
Hi,
First of all thank you for your article. It is my first work with sharp and has been great read it.
On the point 12 "one or more indexers", can it be defined more than one? I have and object with several ArrayList and would be great if I can access to any of them as a matrix.
Could you tell me more?
thans in advace.
|
|
|
|
|
Cosh wrote something like:
On the point 12 "one or more indexers", can it be defined more than one? I have and object with several ArrayList and would be great if I can access to any of them as a matrix.
You can define more than one indexer, as long as they have a different signature. This in contrast to a *real* property, which doesn't have a signature.
The signature of an indexer may have more than one parameter.
We are all men, except for the women.
|
|
|
|
|
Well written and illustrated. Many thanks for this.
Well done!
/**********************************
Paul Evans, Dorset, UK.
Personal Homepage "EnjoySoftware" @
http://www.enjoysoftware.co.uk/
**********************************/
|
|
|
|
|
Just noticed that 15 says "implement IDisposable" but the class MyClass actually doesn't
For any newbies, that should be:
public class MyClass : IDisposable
{
...
}
... also remember that all classes implicitly inherit from object in absence of any explicit base class. Direct Multiple inheritance of classes is sadly not allowed, but you may implement as many interfaces as you like.
Cheers!
/**********************************
Paul Evans, Dorset, UK.
Personal Homepage "EnjoySoftware" @
http://www.enjoysoftware.co.uk/
**********************************/
|
|
|
|
|
Paul Evans wrote:
Just noticed that 15 says "implement IDisposable" but the class MyClass actually doesn't
My bad! Whenever I find time to update this article this will be fixed.
The nice thing about C++ is that only your friends can handle your private parts.
|
|
|
|
|
Thanks!
The nice thing about C++ is that only your friends can handle your private parts.
|
|
|
|
|
I have a question to coding conventions. Should I use the private modifier for class members or not?
At time I dont use it. It looks fine for fields but makes methods and properties a little bit hard to see.
|
|
|
|
|
I always specify the access modifier. I believe it make the intention clearer.
The nice thing about C++ is that only your friends can handle your private parts.
|
|
|
|
|
First of all, thanks for these helpful guidelines.
Now, on item 5 you mention:
Enumerations that represent bit masks should have the [Flags] attribute.
Why? Please expand on this, or at least provide us with a link that explains it.
Also, what's the "proper" way to check if a bit is set? It certainly isn't as clean-looking as it is in C++. Here's what I mean, see if you concur:
C#
class CSharp
{
enum Flags { Nope = 1, Yep, Maybe };
int flags;
...
void foo()
{
if (this.flags & (int)Flags.Yep != 0)
return;
}
}
C++
class CPlusPlus
{
enum Flags { Nope = 1, Yep, Maybe };
int m_flags;
...
void foo()
{
if (m_flags & Yep)
return;
}
}
Thanks,
Alvaro
When birds fly in the right formation, they need only exert half the effort. Even in nature, teamwork results in collective laziness. -- despair.com
|
|
|
|
|
Alvaro Mendez wrote:
Enumerations that represent bit masks should have the [Flags] attribute.
Why? Please expand on this, or at least provide us with a link that explains it.
When an enum uses the [flag] attribute, the debugger will display enum values using the identifiers and not the numeric value. Also, you don't have to cast to int to work with the bitwise operators. See the FlagsAttribute[^] class in MSDN.
The nice thing about C++ is that only your friends can handle your private parts.
|
|
|
|
|
By implementing operator overloading your class is no longer going to be CLS compliant. If you do use operator overloading in your class you should include named methods that offer the same functionality:
public static bool operator<(MyType lhs, MyType rhs)<br />
{}<br />
<br />
public static bool LessThan(MyType lhs, MyType rhs)<br />
{}
Simon
|
|
|
|
|
You are right, operator overloading is not CLS compliant. This is an oversight on my part because items 1 and 4 recomend implementing operators in terms of other functions, which are CLS compliant. Whenever I have a chance I'll correct the article text. Thanks for your comments.
There are only 10 kind of people in the world: those who understand binary and those who don't.
|
|
|
|
|
First of all I thought this article was great.
I have a suggestion relating to the implementation of the IEnumerator interface, section 14. I noticed that you quoted some the text from MSDN regarding when to throw exceptions. If you read the two paragraphs above the one you quoted, there is mention of throwing exceptions on calls to Current when either of two conditions are met. First if Current is called before the initial call MoveNext or after Reset is called an exception should be thrown and second if Current is called after MoveNext has returned false.
I would also suggest adding the initialization of m_index to -1 in the constructor. If this is not done then it would be initialized to 0 and you would be able to call Current immediately after instatiation of the IEnumerator which violates the rule that the Enumerator should be positioned just before the first element in the collection. I'm assuming that your collection is zero based based on the implementation of your Reset method.
Suggestion for modification of Current
public object Current
{
get
{
if(m_index >= m_col.Data.Count)
throw new InvalidOperationException();
if(m_index == -1)
throw new InvalidOperationException();
return m_col.Data[m_index];
}
}
|
|
|
|
|
KenD wrote:
First of all I thought this article was great
Thanks and I'm glad you liked it.
Regarding your suggestions, of course you are right. This article started out as a log I kept while learning C# and thanks to the suggestions of several readers it has grown into a neat collection of really useful guidelines (and I've learned alot in the process!). Unfortunately, I've been really busy and I haven't had a chance to update the article in a while... and it doesn't seem like I can anytime soon. I just hope that the readers go through all the comments and have a chance to spot all the important suggestions and corrections.
There are only 10 kind of people in the world: those who understand binary and those who don't.
|
|
|
|
|
In C# 3.0 and later this happens automatically by the compiler. Actually, tests should be for
default(int) rather than (x < -1).
|
|
|
|
|
Hi,
in your article you suggest to implement operator== in that way:
public static bool operator==(MyType lhs, MyType rhs)
{
if(lhs == null)
return false;
return lhs.Equals(rhs);
}
Actually I think that this is a very dangerous idea. The reason is that you now no longer can check for null references in the the "usual" way. Let's assume a reference type named Foo implements operator == in that way and you write the following code:
Foo f= null;
if(null==f)
{
}
In this case the statement null==f always returns false;
In the same way null!=f always returns true. In other words the operators
work exactly the opposite way as expected.
Therefore I highly recommend to call the static Object.Equals method inside operator == and operator !=. In the documentation of Object.Equals one can read:
Return Value
true if objA is the same instance as objB or if both are null references or if objA.Equals(objB) returns true; otherwise, false.
Therefore I suggest that operator == and operator != is _always_ implemented in that way (I'm still speaking of reference types):
public static bool operator ==(Foo obj1, Foo obj2)
{
return Object.Equals(obj1, obj2);
}
public static bool operator !=(Foo obj1, Foo obj2)
{
return !Object.Equals(obj1, obj2);
}
Even if you override Equals in a very, very specific way I can't think of a good example where comparing an object against null should behave in a completly, IMHO unexpected, different way. I also can't think of a good example where comparing two null references (regardless of wich type they are) should not return true.
And last but not least I can't think of a good reason where comparing one and the same reference should (or even could) return false.
Any thoughts?
"You can always improve your chances of writing
bug-free code by writing code that doesn't do anything"
Rob Macdonald, Serious ADO
|
|
|
|