yep - I was attempting to get rid of the non-local return. additionally
(and this is purely personal preference) I try to avoid matching on options
wherever possible - pattern matching has a very slight overhead, and I tend
to think that map/foreach on an Option tends to look better and be a bit
clearer. with that said, I'm not in love with that part of the code - it is
pretty ugly, but I didn't want to spend more time on it since it works.

Sent from my iPhone

On Apr 30, 2013, at 11:42 PM, Christopher Currie <notifications@github.com>
wrote:

In
src/main/scala/com/fasterxml/jackson/module/scala/introspect/ScalaPropertiesCollector.scala:

>    /*
>     * This is essentially the base class function, except that we don't check
>     * BeanUtils to see if the name is permissible as a setter method (Scala
>     * has different rules there and ScalaBeans has already vetted the method).
>     */
> -  private def _addGetterMethod(prop: POJOPropertyBuilder, explName: Option[String], m: AnnotatedMethod) {
> -    Option(_annotationIntrospector) match {
> -      case Some(ai) => {
> -        if (ai.hasAnyGetterAnnotation(m)) {
> -          anyGetters += m
> -          return
> -        }
> -        if (ai.hasAsValueAnnotation(m)) {
> -          jsonValueGetters += m
> -          return
> -        }
> +  private def _addGetterMethod(implName: String, explName: Option[String], m: AnnotatedMethod) {

I've played around a little with the rewrite here, as there doesn't appear
to be an actual bug in original. Looks like you're going for one (or
possibly both) of two goals here:

1) Eliminate the try/catch overhead of the non-local return (which was
unknown to me until I investigated).
2) Stylistically prefer condition flags to early returns.

I fully support the former and mildly disagree with the latter; I tend to
believe early returns are easier to read than conditions. Unless I'm
missing something, I'll commit this with a small rewrite that handles (1)
but doesn't follow (2).

—
Reply to this email directly or view it on
GitHub<https://github.com/FasterXML/jackson-module-scala/pull/75/files#r4032624>
.