-
-
Notifications
You must be signed in to change notification settings - Fork 743
Added opEquals to InternetAddress #2839
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
* Examples: | ||
* -------------- | ||
* InternetAddress addr1,addr2; | ||
* if (addr1 == addr2) { } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Uhm.. won't this example segfault due to a null this
? I forgot whether this was ever fixed (or ever will be).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tested it and it seems to work. http://dlang.org/operatoroverloading.html
If a and b are both class objects, then the expression is rewritten as:
.object.opEquals(a, b)
and that function is implemented as:bool opEquals(Object a, Object b) { if (a is b) return true; if (a is null || b is null) return false; // NULL HANDLED HERE if (typeid(a) == typeid(b)) return a.opEquals(b); return a.opEquals(b) && b.opEquals(a); }
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok cool so it was fixed, great!
Auto-merge toggled on |
Added opEquals to InternetAddress
Thanks, looks good. |
override bool opEquals(Object o) const | ||
{ | ||
auto other = cast(InternetAddress)o; | ||
return other && this.sin.sin_addr.s_addr == other.sin.sin_addr.s_addr && |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is testing other
for null really necessary?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
other
will be null if o
wasn't an InternetAddress
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a good thing to discuss. It depends on how opEquals is supposed to be used. If it's valid to pass other non-InternetAddress objects then it is necessary. If not, then we could use a contract to validate that o is of type InternetAddress. However, I plan on submitting an enhancement request to change the '==' operator to use a typed opEquals instead of the generic one, which would mean we wouldn't need to cast the Object parameter cause we could just use an InternetAddress directly.
I'm not sure why the Address classes do not have opEquals implemented. I've added it to the InternetAddress class. If this get's "approved" then I can add the other opEquals methods to the other Address classes.