You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@pekko.apache.org by "mdedetrich (via GitHub)" <gi...@apache.org> on 2023/05/09 07:46:00 UTC

[GitHub] [incubator-pekko] mdedetrich commented on a diff in pull request #316: Add OptionConverters.toScala methods for java Optional primitives

mdedetrich commented on code in PR #316:
URL: https://github.com/apache/incubator-pekko/pull/316#discussion_r1188260033


##########
actor/src/main/scala-2.12/org/apache/pekko/util/OptionConverters.scala:
##########
@@ -21,9 +21,21 @@ import java.util._
 @InternalStableApi
 private[pekko] object OptionConverters {
   import scala.compat.java8.OptionConverters.SpecializerOfOptions
+  import scala.compat.java8.OptionConverters._
 
   @inline final def toScala[A](o: Optional[A]): Option[A] = scala.compat.java8.OptionConverters.toScala(o)
 
+  // The rest of the .toScala methods that work with OptionalDouble/OptionalInt/OptionalLong have to be manually
+  // redefined because the scala.compat.java8.OptionConverters.toScala variants work with scala.lang primitive types
+  // where as scala.jdk.javaapi.OptionConverters.toScala works with java.lang primitive types. Since the primary
+  // usecase of these functions is for calling within Java code its preferrable to return Java primitives, see
+  // https://github.com/scala/bug/issues/4214
+  def toScala(o: OptionalDouble): Option[java.lang.Double] = if (o.isPresent) Some(o.getAsDouble) else None

Review Comment:
   I am intentionally not using `@inline` here because its the actual definition of the method being placed here, not another method call. JVM will inline these statements without issues.



##########
actor/src/main/scala-2.12/org/apache/pekko/util/OptionConverters.scala:
##########
@@ -21,9 +21,21 @@ import java.util._
 @InternalStableApi
 private[pekko] object OptionConverters {
   import scala.compat.java8.OptionConverters.SpecializerOfOptions
+  import scala.compat.java8.OptionConverters._
 
   @inline final def toScala[A](o: Optional[A]): Option[A] = scala.compat.java8.OptionConverters.toScala(o)
 
+  // The rest of the .toScala methods that work with OptionalDouble/OptionalInt/OptionalLong have to be manually
+  // redefined because the scala.compat.java8.OptionConverters.toScala variants work with scala.lang primitive types
+  // where as scala.jdk.javaapi.OptionConverters.toScala works with java.lang primitive types. Since the primary
+  // usecase of these functions is for calling within Java code its preferrable to return Java primitives, see
+  // https://github.com/scala/bug/issues/4214
+  def toScala(o: OptionalDouble): Option[java.lang.Double] = if (o.isPresent) Some(o.getAsDouble) else None

Review Comment:
   I am intentionally not using `@inline` here because its the actual definition of the method being placed here, not another method call. JVM should inline these statements without issues.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org