|
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
|
|
|
|
|
You are right, the implementation I described does not work for the "null == obj" kind of comparision. Your suggestion to always use the static Object.Equals() is incorrect because Object.Equals() only checks for reference equality, not value equality. So I believe operator == should be implemented like this:
public static bool operator==(MyType lhs, MyType rhs)
{
if(lhs == null && rhs == null)
return true;
if(lhs == null)
return false;
return lhs.Equals(rhs);
}
Thanks for spotting the error, I'll update the article as soon as possible.
There are only 10 kind of people in the world: those who understand binary and those who don't.
|
|
|
|
|
Hi,
thanks for your comments!
[...] incorrect because Object.Equals() only checks for reference equality, not value equality
Not exactly!
The static version first checks if both object are of one and the same reference. If that's not true it checks weather one of the two objects is a null reference. After that it calls
objA.Equals(objB);
So if you override your Equals method, that method gets eventually called. Only if you don't override the Equals method, objA.Equals(object, object) checks for reference equality due to the fact that Object.Equals(object) does that by default.
example:
using System;
class Foo
{
public Foo(int value){this.Value=value;}
public int Value;
public override bool Equals(object obj)
{
Console.Write("Inside Foo.Equals");
if(obj==null)
return false;
if(this.GetType()!=obj.GetType())
return false;
Foo other=(Foo)obj;
return this.Value==other.Value;
}
public override int GetHashCode()
{
return Value.GetHashCode();
}
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);
}
}
class MainClass
{
public static void Main(string[] args)
{
Foo f1=new Foo(0);
Foo f2=new Foo(1);
Console.Write("f1==f2 : ");
if(f1==f2)
Console.WriteLine("true");
else
Console.WriteLine("false");
Console.Write("f1==null : ");
if(f1==null)
Console.WriteLine("true");
else
Console.WriteLine("false");
}
}
The output is:
f1==f2 : Inside Foo.Equals false
f1==null : false
As you can see, the overridden Equals gets called from the static Object.Equals method.
In essence Object.Equals does exactly what you have written in your last posting.
The benefit is that you get the expected result even when comparing against null reference(s), but one can still provide a very specific function for equality.
cu
Max
"You can always improve your chances of writing
bug-free code by writing code that doesn't do anything"
Rob Macdonald, Serious ADO
|
|
|
|
|
See... it's all your fault! You shouldn't make me respond to your comments that early in the morning! On a more serious note, I haven't used C# in a while and I misread the msdn documentation. The implementation I proposed in the previous message is correct (not very efficient thou), but using the static Object.Equals() saves some typing.
There are only 10 kind of people in the world: those who understand binary and those who don't.
|
|
|
|
|
|
Maximilian Hänel wrote:
calls operator == over and over again and eventually results in a StackOverflowException
See, I just love making a fool out of myself! (This stack overflow problem happened to me, I'm very aware of it and I still make silly comments! ) That's what happens when you try to discuss C# with your C++ hat on! (And when you write code off of the top of your head without testing it first!)
There are only 10 kind of people in the world: those who understand binary and those who don't.
|
|
|
|
|
what happens when you try to discuss C# with your C++ hat on!
ROFLMAO I know what you mean.I just wrote a few lines of C++ code and I realized that
class
{
public void Foo()
{
}
}
doesn't compile for at least two reasons. And all that because I left C++ for only 2 weeks...
"You can always improve your chances of writing
bug-free code by writing code that doesn't do anything"
Rob Macdonald, Serious ADO
|
|
|
|
|
Would I be wrong in pointing out that the above guidelines can also be, and should be, applied to .NET classes written in C++, VB.NET, J# etc. etc.?
Obviously syntax changes but the concepts don't, right?
If so then I shall be updating CP+ Thanks for the tips.
regards,
Paul Watson
Bluegrass
Cape Town, South Africa
|
|
|
|
|
Well, some items are applicable to all .NET languages but, some languages don't support operator overloading, indexers, etc.
All of my opinions are correct, even when reality makes the mistake of disagreeing with me.
ASP.NET can never fail as working with it is like fitting bras to supermodels - it's one pleasure after the next - David Wulff
|
|
|
|
|
[i]All classes should be CLS compliant[/i]
I would say: interfaces of all the public classes should be CLS compliant.
I vote pro drink
|
|
|
|
|
Nemanja Trifunovic wrote:
I would say: interfaces of all the public classes should be CLS compliant.
Well, the term interface has already a well defined meaning in C#. The item should probably be refrased as "All classes, structs and interfaces should be CLS compliant."
All of my opinions are correct, even when reality makes the mistake of disagreeing with me.
ASP.NET can never fail as working with it is like fitting bras to supermodels - it's one pleasure after the next - David Wulff
|
|
|
|
|
FYI:
If after your using statements you add the following line:
[assembly:CLSCompliant(true)]
then when you compile your code the compiler will insure that all of your code is CLS compliant.
|
|
|
|
|
That's exacly what item 8 states. What I was discussing was that Nemanja's use of the word "interface" in a generic sense is somewhat confusing because "interface" already has a well defined meaning in C#.
There are only 10 kind of people in the world: those who understand binary and those who don't.
|
|
|
|
|
How about "all types should be CLS-compliant"? (since a "type" is considered any class, struct, or interface, in C#)
|
|
|
|
|
dacris wrote:
How about "all types should be CLS-compliant"? (since a "type" is considered any class, struct, or interface, in C#)
I agree
The nice thing about C++ is that only your friends can handle your private parts.
|
|
|
|
|
I haven't had time to read through the entire article, but at a glance it looks very good. However, #13 is in error. In that bullet, you state that "For a 'collection' class to be used in a foreach loop it must implement IEnumerable."
Here's the syntax for the foreach operator:
<br />
foreach (type-identifier in expression) <br />
embedded-statement<br />
Technically, the foreach operator will work on expression as long as expression is said to be a collection type. This means that expression must either implement IEnumerable (as you mentioned) or implement the collection pattern, which is defined as follows:
* expression must contain a public instance method called GetEnumerator that takes no parameters and returns a struct, class or interface.
* The type returned from the GetEnumerator method must contain a public instance method called MoveNext that takes no parameters and returns a bool value.
* The type returned from the GetEnumerator method must contain a public instance method called Current that takes no parameters and returns a reference type.
In case you're interested, 'Inside C#' has a full chapter on enumeration which goes into all sorts of various ways of implementing enumerators including creating versioned enumerators, combining the IEnumerable and IEnumerator interfaces, protecting data while allowing enumeration, various performance issues and using a mutator interface to modify collected value types.
Cheers,
Tom Archer
Author - Inside C#, Visual C++.NET Bible
Please remember to keep this for tax purposes.
|
|
|
|
|
Tom Archer wrote:
haven't had time to read through the entire article, but at a glance it looks very good
Thanks
Tom Archer wrote:
However, #13 is in error. In that bullet, you state that "For a 'collection' class to be used in a foreach loop it must implement IEnumerable."
Well, I wouldn't say it's in error, it just doesn't show the whole picture. This article is just a list of reminders of "what should be done" while I learn C#. Trying to explain all the "whys" is out of the scope of the article and out of my current knowledge of C#. There are plenty of great C# books out there that anybody could (and should!) get to reach C# mastery.
Tom Archer wrote:
'Inside C#' has a full chapter on enumeration which goes into all sorts of various ways of implementing enumerators
I've read 'Inside C#' and I must say it's a recommended reading.
Eddie Velasquez: A Squeezed Devil
Checkout General Guidelines for C# Class Implementation
|
|
|
|
|
Please don't take my correction as putting your article down as I think your article is a very good starting point for many people that are new to C#. However, your wording on the foreach/IEnumerable does state that you must implement IEnumerable (as opposed to stating that it is one way).
Thanks for the kind words on the book. Have you read the second edition? It has a lot of depth on issues such as the aforementioned IEnumerable interface.
Cheers,
Tom Archer
Author - Inside C#, Visual C++.NET Bible
Please remember to keep this for tax purposes.
|
|
|
|
|
Tom Archer wrote:
Please don't take my correction as putting your article down
Never took it that way, I'm always open and greatful for constructive critisism, specially from people who know what they're talking about.
Tom Archer wrote:
However, your wording on the foreach/IEnumerable does state that you must implement IEnumerable (as opposed to stating that it is one way)
That's why I said that the item doesn't show the whole picture. As soon as a pending update gets posted I will update the collection-related items to show the a more complete view and some optimizations that can be done when defining and implementing collections and enumerators.
Tom Archer wrote:
Have you read the second edition?
That's the one I was refering to. Great book, one of three that I currently consider must-read C# books.
Eddie Velasquez: A Squeezed Devil
Checkout General Guidelines for C# Class Implementation
|
|
|
|
|
Eddie Velasquez wrote:
That's the one I was refering to. Great book, one of three that I currently consider must-read C# books.
And the other two are? I'm always looking for a good book.
Jerry
|
|
|
|