[
https://issues.apache.org/jira/browse/THRIFT-1871?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13601077#comment-13601077
]
Eirik Sletteberg commented on THRIFT-1871:
------------------------------------------
Maybe it's not very clear, but I'm actually talking about two different, but
related issues:
1) Improving the way clients handle optional fields
2) A proposed improvent to the IDL so that functions may be marked as
optional/nullable
I'll argument for (1) first:
Here's a new, slighty modified example where not all users have specified their
age.
In java, you would do something like this:
Thrift IDL:
-------
struct User {
1: i3d id;
2: optional i32 born;
3: string name;
}
service UserService {
User getUserById(1: i32 id);
}
Java code:
------
User user = myThriftClient.getUserById(123);
int age = 2013 - user.getBorn(); // If age is optional and not specified,
returns zero
System.out.println("The user is "+age+" years old");
-------
The problem is; the java code compiles, but gives us weird errors. If a user
doesn't have age specified, it would print "The user is 2013 years old", and
not "age unknown". Say we generated java code with a new compiler flag that
says "use-option-wrappers-for-optional-fields". Then we would get:
----------
User user = myThriftClient.getUserById(123);
int age = 2013 - user.getBorn(); // Compiler error: Class Option<Integer>
cannot be cast to type integer (or something like that)
----------
This forces the developer to do the null check, for example like this:
----------
User user = myThriftClient.getUserById(123);
if (user.getBorn() instance of Some<int>) {
int age = 2013 - user.getBorn().get();
System.out.println("The user is "+age+" years old.);
} else {
System.out.println("Age not specified.");
}
----------
Of course, if everybody remembers to do their null checks everytime, this won't
be a problem, but the fact is we are humans, we forget null checks, and type
safety is here for our rescue. The more errors our compiler can catch, the less
errors will go into production.
"could the protocols handle these optional results?"
I'm thinking of implementing this as a backwards-compatible change that has no
impact on the protocols/transports. As of today, the protocols handle passing
of null values. The difference lies in the clients' generated code - as of
today, if null values are passed in, developers have to remember their null
checks. With a new compiler flag (specifiyng use of options), one would get
generated code that uses Option wrappers for optional fields - then the
compiler will catch the errors.
"And how would you implement this for all other target languages?"
I would say we could add a compiler flag "use-option-class-for-optional-fields"
or something like that. If this flag is set, compilers will generate code that
has null-safe handling of optional fields, if not, the code generator will not
accept this flag. I believe most languages supported by Thrift have the ability
to implement Option structs in some way or another. (Again; if not, the code
generator doesn't allow this flag and generates code for optional fields just
the same way it does today.)
Regarding case nr. 2:
"Moreover, I don't really see what's the big deal of using either exceptions or
optional fields to handle your example."
In the case that a user with the given ID doesn't exist, one can solve it by
throwing an exception, but that kind of violates the "principle" only use
exceptions for exceptional conditions?
And as of today, all functions are allowed to return null. So if a user is not
found, and the server is implemented to return null, then a client will get a
(unexpected?) NullPointerException (or the equalent in other languages).
> Better null-safe handling
> -------------------------
>
> Key: THRIFT-1871
> URL: https://issues.apache.org/jira/browse/THRIFT-1871
> Project: Thrift
> Issue Type: Brainstorming
> Components: Compiler (General)
> Reporter: Eirik Sletteberg
> Attachments: Add_nullable_modifier_to_functions.patch
>
>
> Optional fields are not really handled well, especially in java clients.
> (NullPointerExceptions everywhere)
> Scrooge (Twitter's alternative Thrift compiler implementation) solves this
> problem by wrapping all optional fields in an Option<T> class. Could this be
> implemented in Thrift?
> As of today, services have no way of returning empty results; for example
> UserService {
> 1: User getUser(1: i32 id)
> }
> cannot return an empty user if the ID is not found.
> There are a few ways to solve this:
> UserService {
> 1: User getUser(1: i32 id) throws UserNotFoundException
> }
> -- or --
> UserService {
> 1: List<User> getUser(1: i32 id)
> }
> where an empty list is returned of no user is found, and a list containing
> one item is returned if a user is found.
> -- or --
> UserResult {
> 1: bool exists;
> 2: User user;
> }
> UserService {
> 1: UserResult getUser(1: i32 id)
> }
> I don't like any of them.
> One could solve this by letting the "optional" keyword apply to methods:
> UserService {
> 1: optional User getUser(1: i32 id)
> }
> or adding a new type to the IDL; the Option type:
> UserService {
> 1: Option<User> getUser(1: i32 id)
> }
> and then, like Scrooge does, generate code that wraps the optional fields in
> an "Option" type, which is null-safe.
> The Option type could also be used in structs:
> struct User {
> 1: i32 id;
> 2: string name;
> 3: optional string email;
> }
> -- or --
> struct User {
> 1: i32 id;
> 2: string name;
> 3: Option<string> email;
> }
> Either way, a null-safe Option wrapper would be used in the generated java
> code.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
