ianlee-dev / facebook-java-api

Automatically exported from code.google.com/p/facebook-java-api
0 stars 0 forks source link

New, simpler behaviour for executeBatch() which removes the need for the RETURN_TYPES introspection #158

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago
The FacebookXXXRestClient types currently perform introspection to work out
what it's method's return types are. They do this so that when
executeBatch() runs, a List of Objects can be returned to the user which
takes into account that some methods are coded in IFacebookRestClient to
return int or String etc.

This can be avoided if the contract for executeBatch() is slightly
redefined (simplified) as follows:
1. FacebookXmlRestClient returns List<Document>. The method doesn't need to
try and work out whether the corresponding method return type is int or
String, it just passes on the list of Documents passed back by Facebook.
2. FacebookJsonRestClient returns List<Object> where each object type for
each individual method call is just what the JSON parser determines it is.
If it sees "a string" it returns String type. If it sees true then it
returns boolean type. If it sees 2 it returns a long type. If it sees 2.3
it returns a double type.
3. FacebookJaxbRestClient returns List<Object> based on how the JAXB
context parses the incoming Document. Users will get ints, booleans and
Strings depending on what's specified in schema.xsd. If the parser can't
figure out what it is (unlikely) then the user will get a
JAXBElement<Object> or just Object.

I've tried this approach in my dev sandbox by just removing all the
RETURN_TYPES code. All JUnit tests pass. To be fair, though, we haven't got
many JUnit tests that look at the result of batch runs.

Comments please. Do you feel that this simplification is justified?

Original issue reported on code.google.com by david.j....@googlemail.com on 13 Jan 2009 at 12:22

GoogleCodeExporter commented 8 years ago
Another alternative to the introspection which I don't quite like as much is to 
add a
return type on ExtensibleClient.callMethod() so that each of the methods like 
Long
marketplace_editListing can, at the time the method is called, specify that 
they want
a Long type returned when the batch is eventually run.

That's more reliable and simpler than doing the introspection. Less prone to 
problems
with multiple definitions of the same overloaded method which we've seen in 
previous
bugs.

However, as I said, I prefer the uber-simple approach of just letting the 
parser for
each return type figure out what the return type should be.

Original comment by david.j....@googlemail.com on 13 Jan 2009 at 12:31

GoogleCodeExporter commented 8 years ago
I've checked in my proposed changes on the composition branch.

Original comment by david.j....@googlemail.com on 14 Jan 2009 at 9:20

GoogleCodeExporter commented 8 years ago
i like the uber-simple approach as well, at least for this case.  that will 
probably
break people's code.. :( probably not too badly though :)

Original comment by fern...@gmail.com on 17 Jan 2009 at 6:11

GoogleCodeExporter commented 8 years ago
I just had to introduce a 2 line hack for the Jaxb client (marked as TODO: 
Issue 156 
with some description). This hack can be removed if / when I make the same sort 
of 
changes that I've made to the composition branch into the trunk.

Original comment by david.j....@googlemail.com on 1 May 2009 at 6:59

GoogleCodeExporter commented 8 years ago
This is now in trunk since we made the "composition" branch the main 
development 
version.

Original comment by david.j....@googlemail.com on 13 Jan 2010 at 8:57