[
https://issues.apache.org/jira/browse/OFBIZ-5004?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14241110#comment-14241110
]
Jacopo Cappellato commented on OFBIZ-5004:
------------------------------------------
The Map interface javadoc states that the get(Object) method:
{quote}
Throws:
ClassCastException - if the key is of an inappropriate type for this map
(optional)
{quote}
ClassCastException is a RuntimeException and, in our case, we could throw it if
the key is not a String and (with a little stretch) if the key is not a valid
field name.
This is similar to the proposal mentioned above of throwing an
IllegalArgumentException, but seems more inline with the Map specification.
What do you think?
> Warning in GenericEntity.get unnecessarily clutters logs
> --------------------------------------------------------
>
> Key: OFBIZ-5004
> URL: https://issues.apache.org/jira/browse/OFBIZ-5004
> Project: OFBiz
> Issue Type: Bug
> Components: framework
> Affects Versions: Trunk
> Reporter: Christoph Neuroth
> Assignee: Jacopo Cappellato
> Attachments: iae.patch
>
>
> This code in GenericEntity.java:
> {code}Debug.logWarning("The field name (or key) [" + name + "] is not valid
> for entity [" + this.getEntityName() + "], printing IllegalArgumentException
> instead of throwing it because Map interface specification does not allow
> throwing that exception.", module);
> {code}
> is really annoying and IMHO it is wrong.
> First, it does not print an exception, it only prints that string with no
> stacktrace (I think that was changed at some point).
> Second, IllegalArgument is a RuntimeException so the interface doesn't really
> need to allow it to be thrown, right?
> Personally, I think the warning is not even justified. We sometimes don't
> know exactly what kind of entity we're dealing with and just check if it has
> that field or not. With this code, to prevent excessive log clutter, we have
> to wrap each call with a "if (it.containsKey())". A java map will just return
> null silently as well, and this behavior is documented in the Map interface.
> If anyone is really worried about accessing fields wrong (your tests should
> catch that error), there could be an extra method "getOrThrow" or something,
> but get should just return the value or null.
> Otherwise, at least throw the exception. Log warnings are usually found while
> monitoring production logs, developers will find this much earlier otherwise.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
