[jira] [Comment Edited] (XERCESJ-1649) Dubious calls to CoreDocumentImpl.isXMLName

["Mukul Gandhi (JIRA)" <xe...@xml.apache.org>]

    [ 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: j-dev-unsubscribe@xerces.apache.org
For additional commands, e-mail: j-dev-help@xerces.apache.org