search

evgenekov / protobuf-csharp-port

Automatically exported from code.google.com/p/protobuf-csharp-port
Other
0 stars 0 forks source link

Feature: Override Builder.Equals #72

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago 
Today, Builder does not override object.Equals. One cannot rely on the result 
of Builder.Equals while comparing it against another builder or a built message.

It would be nice if one could pass a Builder or a built message as parameter of 
Builder.Equals() and get a proper response. Perhaps even a Message.Equals() 
should accept a builder as well.

Original issue reported on code.google.com by igorga...@gmail.com on 21 Feb 2014 at 11:02

GoogleCodeExporter commented 9 years ago 
Patch: 
https://github.com/igorgatis/protobuf-csharp-port/compare/issue72?expand=1

Original comment by igorga...@gmail.com on 24 Mar 2014 at 2:58

GoogleCodeExporter commented 9 years ago 
I'm very nervous of overriding Equals on builders, due to their mutable nature.

I can see it being useful in tests, although you can always call Build instead 
of course. But I really wouldn't want them to be used as keys in a dictionary 
or anything like that.

Let me ponder on this one a bit. (I'll have a look at the Java code as well...)

Original comment by jonathan.skeet on 24 Mar 2014 at 6:51

GoogleCodeExporter commented 9 years ago 
I've had a look at the Java code (I can't easily test it right now) and it 
doesn't override equals/hashCode as far as I can tell. When would you want this 
functionality (other than in tests, where it's not a problem to call Build)?

Original comment by jonathan.skeet on 5 Apr 2014 at 3:14

GoogleCodeExporter commented 9 years ago 
In general, I always like to see a Builder as a "mutable" proto object. I'd 
expect it to work exactly like a proto object except one can change it. It's 
like "const SomeProto&" vs "SomeProto*" in C++ world.

I can think of a couple of scenarios where that would be useful but it feels 
like careful design would suffice to workaround that.

Original comment by igorga...@gmail.com on 7 Apr 2014 at 3:23

GoogleCodeExporter commented 9 years ago 
The problem is that mutable types are fundamentally *not* like immutable types 
when it comes to equality.

In particular, the documentation for Object.Equals(Object) *specifically* says 
it shouldn't be overridden in mutable types:

> You should not override Equals on a mutable reference type. This is because 
overriding Equals requires that you also override the GetHashCode method, as 
discussed in the previous section. This means that the hash code of an instance 
of a mutable reference type can change during its lifetime, which can cause the 
object to be lost in a hash table.

(As an example, StringBuilder doesn't override Equals(Object) - although 
somewhat confusingly, it does add an overload of Equals(StringBuilder).)

Original comment by jonathan.skeet on 7 Apr 2014 at 4:40

GoogleCodeExporter commented 9 years ago 
Perhaps we should close this one. I've used the API for a while now and I don't 
really miss a Builder.Equal method.

Original comment by igorga...@gmail.com on 24 Jul 2014 at 7:36

GoogleCodeExporter commented 9 years ago 
Sounds good to me :)

Original comment by jonathan.skeet on 24 Jul 2014 at 7:43