2
\$\begingroup\$

I'm somewhat new to Scala and am not sure if I'm doing things in a very Scala-esque way.

In particular, I'm not sure if I'm using Option correctly, since by calling .isDefined, I'm basically just doing ye olde Java null check. Also, the getMethodAndAnnotationFromInfaces method could probably be done better.

Please point out ways in which I could better leverage language features in the below code:

for (val method <- clazz.getMethods) {
        val methodAndAnnotation = method.getAnnotation(annotationType) match {
            case annotation: MessageHandler => Some((method, annotation))
            case _ => getMethodAndAnnotationFromInterfaces(clazz.getInterfaces, method, annotationType)
        }

        if (methodAndAnnotation.isDefined){
            // do something
        }
    }

getMethodAndAnnotationFromInterfaces function:

def getMethodAndAnnotationFromInterfaces(interfaces: Array[java.lang.Class[_]], method: Method, annotationType: Class[_ <: java.lang.annotation.Annotation])
    : Option[(Method, java.lang.annotation.Annotation)] = {

    @tailrec
    def getMethodAndAnnotationFromInterfacesInner(interfaces: Array[java.lang.Class[_]], acc: Option[(Method, java.lang.annotation.Annotation)])
        : Option[(Method, java.lang.annotation.Annotation)] = {
        interfaces.length match {
            case 0 => acc
            case _ =>
                try {
                    interfaces.head.getMethod(method.getName, method.getParameterTypes: _*) match {
                        case newMethod: Method =>
                            val newAnnotation = newMethod.getAnnotation(annotationType)
                            if (newAnnotation != null && acc.isDefined) {
                                throw new RuntimeException("The annotation is applied to the method in more than one interface of the class.")
                            }
                            getMethodAndAnnotationFromInterfacesInner(interfaces.tail, Some((newMethod, newAnnotation)))
                        case _ => getMethodAndAnnotationFromInterfacesInner(interfaces.tail, acc)
                    }
                } catch {
                    case e: NoSuchMethodException => getMethodAndAnnotationFromInterfacesInner(interfaces.tail, acc)
                }
        }
    }

    getMethodAndAnnotationFromInterfacesInner(interfaces, None)
}
\$\endgroup\$
1
\$\begingroup\$

For a "scalaesque way" to deal with Option, study Tony Morris' Option Cheat Sheet. Additionally you can use for comprehensions as well (if you don't want to do anything in case of None):

for (value <- optionValue) {...}
\$\endgroup\$
1
\$\begingroup\$

You could write it like this:

for (val method <- clazz.getMethods) {
  val methodAndAnnotation = method.getAnnotation(annotationType) match {
    case annotation: MessageHandler => Some((method, annotation))
    case _                          => getMethodAndAnnotationFromInterfaces(clazz.getInterfaces, method, annotationType)
  }

  methodAndAnnotation match {
    case Some((method, annotation)) => // do something
    case None => //whatever is relevant
  }
}

Or shorter:

for (val method <- clazz.getMethods) {
  method.getAnnotation(annotationType) match {
    case annotation: MessageHandler =>  // do something with method and annotation
    case _                          => {
      val v = getMethodAndAnnotationFromInterfaces(clazz.getInterfaces, method, annotationType)
      //whatever is relevant
    }
  }
}

But it depends on what you want to do / if you have something to do when getMethodAndAnnotationFromInterfaces returns None.

\$\endgroup\$
  • 2
    \$\begingroup\$ Noooooooo! Not matching! \$\endgroup\$ – Daniel C. Sobral Oct 19 '11 at 22:51
  • \$\begingroup\$ @Daniel - Why not matching? I'm a newbie. Odersky's book teaches handing Options with matching. \$\endgroup\$ – Ed Staub Feb 12 '12 at 22:01
  • \$\begingroup\$ @EdStaub I have to kind of retract that. Matching will give you the most speed in a type-safe way. It's verbose, though, and can get you in the bad habit of matching everything, which leads to less composable code. \$\endgroup\$ – Daniel C. Sobral Feb 13 '12 at 3:42
  • \$\begingroup\$ Good. It was not that bad ;-) \$\endgroup\$ – blackbox Nov 12 '13 at 17:34
0
\$\begingroup\$

Another improvent is to use Scala Reflection wihch comes in 2.10:

import scala.reflect.runtime.Mirror
val c = Mirror.classToType(classOf[X])
val members = c.parents map { _.members filter (_.isMethod) }
val ann = Mirror.classToSymbol(classOf[Ann])
val meths = members map { _ filter (_ hasAnnotation ann) }

The output is not perfect yet, but the code is short, clear Scala and prints the expected result:

meths: List[List[scala.reflect.runtime.Mirror.Symbol]] = List(List(), List(method c, method a), List())

I tested this with following code:

// Java
import java.lang.annotation.*;

@Retention(RetentionPolicy.RUNTIME)
@Target(ElementType.METHOD)
public @interface Ann {}

// Scala
trait T {
  @Ann
  def a: Int
  def b(i: Int): String
  @Ann
  def c(s: String): String
}
class X extends T {
  def a: Int = 5
  def b(i: Int) = i.toString
  def c(s: String) = "("+s+")"
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.