Your patch is to the JavaME generator - is this intentional? I'm not
familiar with whether valueOf is supported for that environment, but I'd
welcome a ticket+patch for it if that was desired behavior.

The regular java library already has this issue fixed as of 0.7:
https://issues.apache.org/jira/browse/THRIFT-997.

On Sun, Feb 27, 2011 at 3:54 PM, Niraj Tolia <[email protected]> wrote:

> Is there a particular reason why Thrift uses "new Primitive()" (e.g.,
> "new Integer()") instead of the more efficient "Primitive.valueOf()"
> (e.g., "Integer.valueOf()"). When I go through the Javadocs for all
> the Primitives, they introduced .valueOf() for the corresponding
> primitive in the following Java versions:
>
> Boolean: 1.4
> Byte: 1.5
> Short: 1.5
> Integer: 1.5
> Long: 1.5
> Double 1.5
>
> Given that Thrift requires Java 1.5+, would a patch like the following
> be acceptable? I tested this against some of my sample code and with
> svn r1075121. 'make check' passes as well as my sample code tests but
> my code seem to only be using Integer.valueOf(). Would be happy to
> test against anything else.
>
> I would be happy to open a new issue and attach the patch there.
>
> Index: compiler/cpp/src/generate/t_javame_generator.cc
> ===================================================================
> --- compiler/cpp/src/generate/t_javame_generator.cc     (revision 1075177)
> +++ compiler/cpp/src/generate/t_javame_generator.cc     (working copy)
> @@ -591,17 +591,17 @@
>   if (type->is_base_type()) {
>     switch (((t_base_type*)type)->get_base()) {
>     case t_base_type::TYPE_BOOL:
> -      return "new Boolean(" + value + ")";
> +      return "Boolean.valueOf(" + value + ")";
>     case t_base_type::TYPE_BYTE:
> -      return "new Byte(" + value + ")";
> +      return "Byte.valueOf(" + value + ")";
>     case t_base_type::TYPE_I16:
> -      return "new Short(" + value + ")";
> +      return "Short.valueOf(" + value + ")";
>     case t_base_type::TYPE_I32:
> -      return "new Integer(" + value + ")";
> +      return "Integer.valueOf(" + value + ")";
>     case t_base_type::TYPE_I64:
> -      return "new Long(" + value + ")";
> +      return "Long.valueOf(" + value + ")";
>     case t_base_type::TYPE_DOUBLE:
> -      return "new Double(" + value + ")";
> +      return "Double.valueOf(" + value + ")";
>     default:
>       break;
>     }
>

Reply via email to