[SNMP4J] Unnecessary object creation in MessageDispatcherImpl

Chris Janicki Chris.Janicki at Augur.com
Tue Nov 26 04:58:36 CET 2013


Hi,

I've been tinkering with SNMP4J 2.  Nice work!  I have three miscellaneous observations, if you're interested:

1) Regarding SNMP4J 2, org.snmp4j.MessageDispatcherImpl line# 465 creates: new OctetString(securityName),  But that OctetString is already available in target.getSecurityName(), as seen at line# 406.  Unless I'm missing some threading reason why you want a new instance, could it be a tiny bit better to reuse the object?  

2) Unless it's an RFC requirement, I don't really understand why the Target requires the SecurityLevel?... It seems redundant since the SecurityName points to a User with certain security credentials.  Would you ever specify a SecurityLevel that is different than what the User's credentials (authProtocol/authPassword and privProtocol/privPassword) contains?  Would it be an improvement to simply infer the SecurityLevel by whether or not the User has non-null auth and/or priv settings defined?

3) In the javadoc of CommunityTarget.setCommunity(), it would be nice to explain that this is just a convenience method for setSecurityName().  Also, getCommunity()'s javadoc currently states that the reply is never 'null', but it can be if the default CommunityTarget() constructor is used, or if the superclass's setSecurityName() is used to set it to null.  (But please don't change AbstractTarget.setSecurityName() to reject 'null', because I'm currently using that null as a "flag" to infer that my app should simply reference the securityName of related incoming trap rather than use some pre-set value assigned to the Target.)

I hope that's helpful.
Chris


More information about the SNMP4J mailing list