[jira] Commented: (SM-220) A number of minor code review issues

["Jamie Goodyear (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/activemq/browse/SM-220?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_38904 ] 

Jamie Goodyear commented on SM-220:
-----------------------------------


 Hi All,

  I've reviewed the collection of minor issues described in this
 JIRA entry and have prepared a patch to resolve the remaining
 out standing issues (SM-220-fixes.patch). 

 Cheers,
 Jamie

 Issues 1,2,3: Corrected typos.
 Issue 4: Leaving getTransactionManager as public method.
 Issue 5: This was corrected in a previous revision.
 Issue 6: This was corrected in a previous revision.
 Issue 7: Modified second logging message to capture e1 message.
 Issue 8: This was corrected in a previous revision.


 Files Modified:

 core/servicemix-core/src/main/java/org/apache/servicemix/jbi/messaging/ExchangePacket.java
 core/servicemix-core/src/main/java/org/apache/servicemix/jbi/messaging/MessageExchangeImpl.java
 core/servicemix-core/src/main/java/org/apache/servicemix/jbi/container/JBIContainer.java

> A number of minor code review issues
> ------------------------------------
>
>                 Key: SM-220
>                 URL: https://issues.apache.org/activemq/browse/SM-220
>             Project: ServiceMix
>          Issue Type: Bug
>          Components: servicemix-core
>    Affects Versions: 2.0.2
>            Reporter: Peter Smith
>            Priority: Minor
>
> #1 ExhchangePacket - typo
> 	* @return the proerty from the exchange
> =====
> #2 MessageExhangeImpl - typo
> 	* @return the proerty from the exchange
> =====
> #3 JBIContainer - typo
> 	* light weight initialization - default values for mbeanSErver, TransactionManager etc are null
> =====
> #4 JCAFlow - method modifier
> 	public TransactionManager getTransactionManager() {
> why is this method public?
> =====
> #5 ManagementContext - member default value
> 	private String jndiPath = "/jmxconnector"; 
> Isn't this value unconditionally overwritten every time during init?
> =====
> #6 SedaFlow - Use of JTA_TRANSACTION_NAME
> In doSend instead of:
> 	if (me.getProperty(MessageExchange.JTA_TRANSACTION_PROPERTY_NAME) == null &&...
> could say:
> 	if (!me.isTransacted() &&...
> 	
> =====
> #7 JBIContainer - getTransactionManager
> I think the 2nd JNDI lookup attempt is logging the wrong message if it fails.
> 	log.debug("No  transaction manager found from naming context", e);
> 	
> Shouldn't that be:
> 	log.debug("No  transaction manager found from naming context", e1); // Note: e1 instead of e		
> ======	
> #8 MessageExchangeImpl - setMessage
> There seems inefficient checking of String name. (eg if IN.equals(name) is true no need to keep testing name)
>         if (IN.equals(name) && !can(CAN_SET_IN_MSG)) {
>             throw new MessagingException("In not supported");
>         }
>         if (OUT.equals(name) && !can(CAN_SET_OUT_MSG)) {
>             throw new MessagingException("Out not supported");
>         }
>         if (FAULT.equals(name) && !can(CAN_SET_FAULT_MSG)) {
>             throw new MessagingException("Fault not supported");
>         }
>         if (FAULT.equals(name) && !(message instanceof Fault)) {
>             throw new MessagingException("Setting fault, but message is not a fault");
>         }
>         
> =======
> [END]

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.