[Issue 54] Removed always-true if and unreachable code in XObject.cs

[andi9310]

Removed always-true if and unreachable code in XObject.cs

[andi9310]

@davkean , thanks for response. As of I understand this code, "IEnumerable Annotations() "method can return IEnumerable with contents, empty IEnumerable, but never null. So "o.Annotations() != null" will always be true. I don't know really if it is supposed to work this way. Maybe It should be "o.Annotations() .Any()" here? As far as i know yielding methods are meant to be used with foreach statements etc. so returning null from them would end with NullReferenceExceptions.

[davkean]

Yep, you are correct that o.Annotations<XObjectChangeAnnotation>() != null will never return null. Looks like they were really supposed to use the non-plural version; o.Annotation<XObjectChangeAnnotation> != null.

However, before we remove or change the check - this particular code was written ~8 years ago, I'd like to get an understanding of the behavior that is broken by that bad check - ie some test cases or code samples that show that fixing this gives a better, but still compatible behavior. For example, are we now firing Changing/Changed events when we're not supposed to?

[davkean]

We appreciate the pull request, however, we've very wary about changing/removing code without tests to prove that an issue exists. Feel free to submit another pull request when you have coverage that covers and proves the bug.