zhanglong2005 / funcito

Automatically exported from code.google.com/p/funcito
0 stars 0 forks source link

support chained method calls where possible #45

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
I am wondering whether I could extend the capabilities of , so that we could 
build chained calls as Functions?  In my mind, I think it seems possible.  
Example of what it would look like:

public class MyClass1 {
    public MyClass2 getMC2() {...}
}
public class MyClass2 {
    public String getStringVal() {...}
}

MyClass1 myClass1Stub = callsTo(MyClass1.class);
Function<MyClass1,String> chained = 
functionFor(myClass1Stub.getMC2().getStringVal());

The obvious limitations are that the return-type of the first method call would 
itself have to be proxyable (non-primitive, non-final, etc.).  This leads to 
some questions about whether or not to try this:

I don't see a way for Funcito to detect improper calls on non-proxyable return 
types.  In those cases, the proxy handlers (i.e. CglibMethodInterceptor, etc.) 
would have to continue to return null, and the chained call would result in a 
NPE.  Or else the proxy handler could use Objenesis to return some "default" 
instance of the non-proxyable return type, and then the chained call would have 
some unpredictable side-effects *and* you could silently get an incorrect 
Function.  (Trying to build Functions around expressions that invoke Java 
language operators already has this problem of silent failures.)

So I am not sure whether either of those options is viable and worth the 
potential usage confusion.  Up till now, Funcito has been developed with a goal 
of being as helpful as possible in detecting or preventing mis-use of the API.  
But I don't see a way around this potential mis-use if we implement limited 
chaining support.

Original issue reported on code.google.com by kandpwel...@gmail.com on 23 Jan 2012 at 8:44

GoogleCodeExporter commented 9 years ago

Original comment by kandpwel...@gmail.com on 3 Feb 2012 at 6:38

GoogleCodeExporter commented 9 years ago
And I would especially love if such a chained call could be null-safe.

Original comment by simsc...@gmail.com on 15 Mar 2012 at 10:39

GoogleCodeExporter commented 9 years ago
Null safe in what way?  Are you referring to the first call in the chain 
returning null, or are you referring to the above-mentioned possibility of 
having non-proxyable (i.e. final) return types from the first call?

In the above example above, if .getMC2() returned null, how would you expect 
chained.apply(someClass1) to respond?  I would think I would have to make it 
throw some sort of RuntimeException (probably FuncitoException).  Otherwise, 
you could end up with a Function that didn't truly reflect the actual effect of 
the chained method calls.

We could create a .safe() method to append to the resulting Function that 
captures FuncitoException (or a specific subset), and has the Function return 
null.  Possible -- but still kind of ugly in my mind because you are really 
altering the meaning of the Function to map to something quite different than 
the mere method call chain.  We would then also have to weight the advantages 
of the "Funcito way" at that point against the "standard way" of anonymous 
inner classes.  Would it really feel all that more simple to call:

MyClass1 myClass1Stub = callsTo(MyClass1.class);
Function<MyClass1,String> chained = 
functionFor(myClass1Stub.getMC2().getStringVal()).safe();

I don't know... *maybe* it would.  Let me know what you think, or if I was on 
the wrong track in answer to your proposal.  That might also belong as an 
enhancement request to Guava or the other FP-libs, to have a transformation 
from a Function to a NPE-safe version of the Function, although that would be 
more general (and potentially more unpredictable) than the case of specifically 
getting a NPE in a well-defined chain.

Original comment by kandpwel...@gmail.com on 15 Mar 2012 at 2:48

GoogleCodeExporter commented 9 years ago
Yeah, you nailed it quite right. I was thinking along this track. I often miss 
the "generous" behaviour of java EL in "real" Java programming.

e.g. a Chained Property Call like myObject.getX().getY().getZ() (where all 
get...() calls might return null) is very expensive to write in "Null-Safe" and 
Type-Safe java. Something like

if(myObject == null) return null;
X x = myObject.getX();
if(x == null) return null;
Y y = x.getY();
if(y == null) return null;
Z z = y.getZ();
return z;

in EL, however, it looks easy like in 

#{myObject.x.y.z}

which returns null, whenever myObject, x, y or z are null.

I would love funcito to add some of this simplicity to my Java code. Maybe 
something along the way you proposed?

Original comment by simsc...@gmail.com on 15 Mar 2012 at 6:41

GoogleCodeExporter commented 9 years ago
Another consideration: this suggested addition obviously breaks the Law Of 
Demeter.  I'm not sure how religious I am about keeping that "law", especially 
with methods that are so ubiquitous that they are almost part of the language 
(e.g., "toString()" ).  But there would definitely be detractors of this 
feature who see it as encouraging more "smelly" code.  I could certainly 
document this clearly (as Mockito does with features that they say should have 
limited use, etc.), but it is still worth considering whether or not to 
proceed, in light of violating this principle.

Original comment by kandpwel...@gmail.com on 9 Apr 2012 at 7:14

GoogleCodeExporter commented 9 years ago
That is a legitimate argument and I quite agree that there are many cases where 
such requirements show bad design. However, in my current usecase the objects I 
am operating on are TOs (TransferObjects) that have been genereated by jax-ws. 
In this case, the benefit of re-using small objects in the webservice interface 
outweighs the cost of breaking the Law of Demeter. I could also think of other 
cases where this might be the case: JPA entities, Hibernate objects, ... almost 
all cases where you store data in a composition of value objects.

Original comment by simsc...@gmail.com on 10 Apr 2012 at 6:50

GoogleCodeExporter commented 9 years ago
I just did some more digging into opinions and extrapolation of LoD, and I 
concluded that there are plenty of valid exceptions.  For instance, ubiquitous 
operations on things like Strings and primitive wrappers, DSLs, fluent 
interfaces, frameworks like Hibernate (or JAX-WS) that pretty much "force" the 
creation of POJO hierarchies/graphs... lots of other cases.  That doesn't mean 
that LoD can be ignored, but there are plenty of cases which call for violating 
some of its more simplistic and less-nuanced definitions ("only one dot").

Original comment by kandpwel...@gmail.com on 10 Apr 2012 at 6:16

GoogleCodeExporter commented 9 years ago
Hopefully, no technical issues will prevent this from working, but I have 
started working towards making this happen.  I hope to get it in 1.1, but we 
will see.

Original comment by kandpwel...@gmail.com on 31 May 2012 at 2:59

GoogleCodeExporter commented 9 years ago
w00t!! *fingers crossed*

Original comment by simsc...@gmail.com on 31 May 2012 at 5:42

GoogleCodeExporter commented 9 years ago
Cautiously optimistic...  I don't quite have it working yet, but almost all of 
the pieces are in place to work with cglib.  Even after I have proven this is 
possible, there will be a lot of test code to write (I know... I should be 
writing test first), and then I need to add this to javassist and Java dynamic 
proxies, and more tests.  But I am now betting on this making it into 1.1

Original comment by kandpwel...@gmail.com on 12 Jun 2012 at 2:11

GoogleCodeExporter commented 9 years ago
Even more optimistic... I have it working for Guava Functions in Cglib, and 
maybe also for Javassist.  Should be extremely easy to port to the other 
FP-frameworks, and relatively easy to add Java Dynamic Proxy proxying support 
of chains (not just Cglib and Javassist).  There is still a whole lot of 
testing to do.  Simon, if you want to to help me flush out problems by testing 
*very early* private versions, let me know.  I probably won't release anything 
publicly as Beta since I haven't even finalized the feature set for 1.1.

Original comment by kandpwel...@gmail.com on 15 Jun 2012 at 4:46

GoogleCodeExporter commented 9 years ago
That sounds very promising! Of course I will help by alpha-testing this new 
feature! :)

Original comment by simsc...@gmail.com on 15 Jun 2012 at 11:30

GoogleCodeExporter commented 9 years ago
Just a note for the functionaljava part: F<A,B> has a convenient andThen method 
to compose functions. So I feel this feature less required for that framework.

Original comment by palotai....@gmail.com on 15 Jun 2012 at 12:33

GoogleCodeExporter commented 9 years ago
True, Robin, but I think it would be pretty ugly to have to wrap 2 methods 
independently with Funcito and then join them w/ functionaljava andThen, 
compared to simply building the already chained call:

F<A,B> myFunc = fFor(callsTo(A.class).methodX().methodB());

versus

F<A,X> aToXFunc = fFor(callsTo(A.class).methodX());
F<X,B> xToBFunc = fFor(callsTo(X.class).methodB());
F<A,B> myFunc = aToXFunc.andThen(xToBFunc);

So I think Functionaljava users will find it quite useful as well.

Original comment by kandpwel...@gmail.com on 15 Jun 2012 at 3:47

GoogleCodeExporter commented 9 years ago
Simon (or anybody else), there is an alpha build available at 
https://funcito.ci.cloudbees.com/job/funcito/lastSuccessfulBuild/artifact/build/
libs/funcito-SNAPSHOT-103_18-Jun-2012.jar (binary jar) and 
https://funcito.ci.cloudbees.com/job/funcito/lastSuccessfulBuild/artifact/build/
libs/funcito-src-SNAPSHOT-103_18-Jun-2012.jar (source jar).

The main thing I am interested in testing is the method chaining.  As mentioned 
in my notes above, the main limitation is that any interim method calls before 
the last one have to have a declared return type which is proxyable (i.e., not 
final or primitive).  Thus even if an interim method call in the chain returns 
a non-proxyable type (such as String) which *does* have an alternate interface 
that is proxyable (String implements CharacterSequence), the chain is *still* 
not supported.

I have not yet added in support for Java Dynamic Proxy method chaining.   It 
won't be hard to do so, but I wanted to get this out there quickly.  So for now 
you have to use cglib or javassist.  I will let you know when Java Dynamic 
Proxy support is in.

I know I don't have all of the unit tests I want, but this was a *massive* 
refactoring to make chaining work, so it was hard to see all of the tests I 
wanted to have before I made every change.

The other major feature besides method-chaining is wrapping of methods with 
arguments.  The caveat in this case is that we only support arity-1 functional 
types, so static argument values have to be bound in when the wrapping is being 
done.  I for sure will *not* be considering functional types with arity > 1 and 
deferred argument binding in this release (1.1) -- *maybe* a future release.

Finally, I have switched warnings from System.error to JUL logging.

Questions? I'm happy to answer, but first try looking at the tests in 
FuncitoGuavaFunction_UT.java and FuncitoGuavaPredicate_UT.java for examples of 
the new features.

Original comment by kandpwel...@gmail.com on 19 Jun 2012 at 5:08

GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
I found another limitation in chaining: return types that are generic type 
variables may not be chained because of type-erasure.  Currently, I broke even 
existing (i.e. non-chained) method wrappers around methods that have generic 
type variables return types.  See FuncitoGuavaPredicate_UT.java 
(http://code.google.com/p/funcito/source/browse/test/org/funcito/FuncitoGuavaPre
dicate_UT.java) test for an example test that is broken right now (currently 
annotated with @Ignore).  But I think there is a way to restore that 
functionality.  But because of the erasure, we will never get to chain on the 
end of a Type Variable return type, such as:
class List<T> {
    T get(int);
}
The runtime sees that as "Object get(int);" with a return type of Object, so 
when I go to proxy the return type to support chaining, we get a proxied Object 
rather than a proxied List, which causes a class cast exception even when we 
*aren't* doing chaining.  I will have to detect that the return type is a 
TypeVariable, and return null (like I used to) rather than proxying the return 
object.  This should fix the non-chained instance.  Chained instances would 
result in a NPE, and I will have to document this.

Original comment by kandpwel...@gmail.com on 20 Jun 2012 at 6:06

GoogleCodeExporter commented 9 years ago
... I misstated the above a bit, where I said "The runtime sees that as "Object 
get(int);" with a return type of Object, so when I go to proxy the return type 
to support chaining, we get a proxied Object rather than a proxied List..."

The last part of that should have said: "we get a proxied Object.class rather 
than a proxied <T>.class."

And I fixed the bug I described above for non-chained invocation returning a 
pure generic type <T>.  Chaining with generic intermediate return types will 
still never work.

Original comment by kandpwel...@gmail.com on 22 Jun 2012 at 6:28

GoogleCodeExporter commented 9 years ago
done

Original comment by kandpwel...@gmail.com on 19 Jul 2012 at 8:21

GoogleCodeExporter commented 9 years ago
split off "null-safe" version as Issue 58.

Original comment by kandpwel...@gmail.com on 19 Jul 2012 at 8:43

GoogleCodeExporter commented 9 years ago

Original comment by kandpwel...@gmail.com on 25 Jul 2012 at 2:38