Yeah, looks right. Can we get this attached to an issue so we have the license stuff squared away?

-Bryan

On Jul 13, 2009, at 4:24 PM, David Reiss wrote:

Makes sense to me.  Bryan?

Chengdu Huang wrote:
The change is really simple. I'm attaching the patch and pasting it below:

Index: compiler/cpp/src/generate/t_java_generator.cc
===================================================================
--- compiler/cpp/src/generate/t_java_generator.cc (revision 792770) +++ compiler/cpp/src/generate/t_java_generator.cc (working copy)
@@ -1096,7 +1096,7 @@
         indent() << "if (this." << (*f_iter)->get_name() << " !=
null) {" << endl;
       indent_up();
     }
- bool optional = bean_style_ && (*f_iter)->get_req() == t_field::T_OPTIONAL;
+    bool optional = (*f_iter)->get_req() == t_field::T_OPTIONAL;
     if (optional) {
       indent(out) << "if (" << generate_isset_check((*f_iter)) << ")
{" << endl;
       indent_up();

Chengdu

On Thu, Jul 2, 2009 at 10:00 AM, David Reiss<[email protected]> wrote:
Yeah, we never got around to implementing optional for the normal
Java generated code. If you submit a patch for it, it would probably
be accepted.

--David

Bryan Duxbury wrote:
This is not a "bug" per se. It's a known fact that it does this.

For anything beyond trivial uses, I think you want to use the beans-
style generator. I would personally go so far as to say that we
shouldn't even have the default generator, but that's just me.

-Bryan

On Jul 1, 2009, at 5:15 PM, Chengdu Huang wrote:

I have a simple thrift file:

struct UserDataContent {
    1: i32 id,
    2: optional i32 value,
}

Java file generated by "thrift --gen java " doesn't seem to respect
the "optional" keyword:

  public void write(TProtocol oprot) throws TException {
    validate();

    oprot.writeStructBegin(STRUCT_DESC);
    oprot.writeFieldBegin(ID_FIELD_DESC);
    oprot.writeI32(this.id);
    oprot.writeFieldEnd();
    oprot.writeFieldBegin(VALUE_FIELD_DESC);
    oprot.writeI32(this.value);
    oprot.writeFieldEnd();
    oprot.writeFieldStop();
    oprot.writeStructEnd();
  }

However, java file generated using "thrift --gen java:beans" looks
correct to me:
 public void write(TProtocol oprot) throws TException {
    validate();

    oprot.writeStructBegin(STRUCT_DESC);
    oprot.writeFieldBegin(ID_FIELD_DESC);
    oprot.writeI32(this.id);
    oprot.writeFieldEnd();
    if (isSetValue()) {
      oprot.writeFieldBegin(VALUE_FIELD_DESC);
      oprot.writeI32(this.value);
      oprot.writeFieldEnd();
    }
    oprot.writeFieldStop();
    oprot.writeStructEnd();
  }

My question is that why "normal" java style files should be different
from "beans" style files in terms of handling optional fields?

The change is introduced in rev665308 (t_java_generator.cc:1105).

Thanks,
Chengdu

Reply via email to