You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@thrift.apache.org by Niraj Tolia <nt...@maginatics.com> on 2011/02/28 00:54:30 UTC
new Primitive vs Primitive.valueOf()
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;
}
Re: new Primitive vs Primitive.valueOf()
Posted by Niraj Tolia <nt...@maginatics.com>.
On Sun, Feb 27, 2011 at 4:06 PM, Bryan Duxbury <br...@rapleaf.com> wrote:
> 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.
>
Hmm. I missed that commit and I think that would fix the issue. I am
still learning my way around the thrift codebase and might have gotten
confused between the 0.6 codebase (that, obviously, didn't have the
above commit) and whats currently in SVN.
Further it looks like valueOf() doesn't really make sense for the ME
environment and so, please ignore the noise.
Apologies,
Niraj
> On Sun, Feb 27, 2011 at 3:54 PM, Niraj Tolia <nt...@maginatics.com> 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;
>> }
>>
>
Re: new Primitive vs Primitive.valueOf()
Posted by Bryan Duxbury <br...@rapleaf.com>.
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 <nt...@maginatics.com> 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;
> }
>