[ https://issues.apache.org/jira/browse/VELOCITY-467?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12624548#action_12624548 ]

Adrian Tarau commented on VELOCITY-467:
---------------------------------------

Looking at 102 I've notices a change in logging(from error -> debug) and I have a few comments on public Iterator getIterator(Object obj, Info i)

Since it is a part of a public API obj should be checked for NULL and this method should return an empty iterator implementation and not NULL(static instance, always the same).#foreach does saves some CPU when iterator is null, but most of the time we expect a non-null interator.

Checking for a possible Iterable with Method iter = type.getMethod("iterator", null); should log at least a warning in case of a wrong return type, or an error as it was before. A nicer way it will be to scan for Iterable and use the method cache. Somebody could have an "iterator" method but designed to do something else.

Also at the end, it should log the message "Could not determine type of iterator in #foreach loop at" as a warning at least, I would say error or exception We really want to point that the loop will not be executed probably because a wrong type. There is only one case when the loop should be skipped without a warning and this is when obj == NULL.

> Throw more exceptions and log less errors
> -----------------------------------------
>
>                 Key: VELOCITY-467
>                 URL: https://issues.apache.org/jira/browse/VELOCITY-467
>             Project: Velocity
>          Issue Type: Improvement
>          Components: Engine
>    Affects Versions: 1.5 beta1
>            Reporter: Will Glass-Husain
>            Priority: Minor
>             Fix For: 1.6
>
>
> Now that Velocity application exceptions are based on RuntimeException, we have more opportunity to use exceptions to signal application level problems.  I'm particularly concerned about initialization problems that are logged and may be missed.  We need to review all logged error messages and see if it would be more appropriate to throw an exception instead.  Some of these places we may need to leave as is for backwards compatibility reasons.  (e.g. macro in the global macro library doesn't parse properly).
> Llewellyn Falco made a good case for this on the dev list recently:
> http://www.mail-archive.com/velocity-dev@jaka.../msg15067.html
> #####
> I still would like to put in my vote that sending error's to the log is an incredibly BAD idea.
> If something is not working, it should be LOUDLY shown as an exception.
> If it is working I don't really need a log.
> The (velocity) log should be there for velocity developers (those programming the actual velocity code) not users.
> I don't ever care to see tomcat's log, I care to see the things I log while in tomcat.
> Most of all, many many many people do not check the log at all, let alone frequently.
> ####

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


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@velo...
For additional commands, e-mail: dev-help@velo...