[
https://issues.apache.org/jira/browse/XERCESJ-1649?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14193739#comment-14193739
]
Mukul Gandhi edited comment on XERCESJ-1649 at 11/2/14 6:55 AM:
----------------------------------------------------------------
your analysis without knowing the semantic and all other intents of the code is
probably correct. but there may be other sides of the story, for example,
1) the calls you cited return a boolean value which is not used, looks like a
redundant statement. may be the calls raise an exception which is thought to
implement a control flow (although not a best practice), in which case we need
to retain these calls.
2) the original authors of the code thought, they would come back to these
areas of code for some reason but still haven't been revisited due to
constraints which are unexplainable to us, in which case also we ought to
retain the code.
I also think the issue type needs to be changed to improvement rather than a
bug.
was (Author: mukul_gandhi):
your analysis without knowing the semantic and all other intents of the code is
probably correct. but there may be other sides of the story, for example,
1) the calls you cited return a boolean value which is not used, looks like a
redundant statement. may be the calls raise an exception which is thought to
implement a control flow (although not a best practice), in which case we need
to retain these calls.
2) the original authors of the code thought, they would come back to these
areas of code for some reason but still haven't been revisited due to
constraints which are unexplainable to us, in which case also we ought to
retail the code.
I also think the issue type needs to be changed to improvement rather than a
bug.
> Dubious calls to CoreDocumentImpl.isXMLName
> -------------------------------------------
>
> Key: XERCESJ-1649
> URL: https://issues.apache.org/jira/browse/XERCESJ-1649
> Project: Xerces2-J
> Issue Type: Bug
> Affects Versions: 2.11.0
> Reporter: Tagir Valeev
> Priority: Minor
>
> I was just analyzing the Xerces source code (ver 2.11.0) and found some
> dubious code.
> First, in class org.apache.xerces.dom.DOMNormalizer in method protected Node
> normalizeNode (Node node) there's a branch (around line #495):
> {noformat}
> if (fDocument.errorChecking && ((fConfiguration.features &
> DOMConfigurationImpl.WELLFORMED) != 0) &&
> fDocument.isXMLVersionChanged()){
> CoreDocumentImpl.isXMLName(node.getNodeName() ,
> fDocument.isXML11Version());
> }
> {noformat}
> However CoreDocumentImpl.isXMLName seems to have no side effect, just return
> the boolean value which is ignored. Probably adding DOM error was forgotten
> here?
> The similar case is in org.apache.xml.serialize.DOMSerializerImpl, method
> private void verify (Node node, boolean verifyNames, boolean xml11Version),
> around line #1000:
> {noformat}
> case Node.ENTITY_REFERENCE_NODE: {
> // only if entity is preserved in the tree
> if (verifyNames && (features & ENTITIES) != 0){
> CoreDocumentImpl.isXMLName(node.getNodeName() , xml11Version);
> }
> break;
> }
> {noformat}
> Probably it's not so big issue, I just wanted to draw your attention to this
> code.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
