mktany2k / funcito

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

single package for client API ? #2

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
I think it is important to have the client-facing API in a single package, with 
nothing else in it.

It should probably be org.funcito, though perhaps org.funcito.api

The main benefit is that the code becomes abundantly clear with respect to:

(a) the client-side API (which is a serious contract)
(b) the internals (which may change as necessary, so long as tests pass)

Original issue reported on code.google.com by codeto...@gmail.com on 19 Nov 2011 at 7:00

GoogleCodeExporter commented 9 years ago

Original comment by kandpwel...@gmail.com on 19 Nov 2011 at 11:36

GoogleCodeExporter commented 9 years ago
I guess I don't understand the comment here. It just says "1" but the list-view 
of Issues says it is Accepted.

Does this mean "go ahead" ? The Issues are growing on me (versus a simple txt 
file) but there is a lot of this interface that is confusing.

Right now, I am tempted to make a branch and go nuts with it. Do everything 
I've been thinking of, and have you review it. If we like it, great; if not, we 
can pull what we want from it.

Original comment by codeto...@gmail.com on 22 Nov 2011 at 10:40

GoogleCodeExporter commented 9 years ago
I just changed the status on several items to "Accepted" without adding 
comments.  Hold off a day or two for the branch thing - I am working on 
removing Invoker. I may actually get it in this morning before I leave for 
work, but I don't know if I have time.

Original comment by kandpwel...@gmail.com on 22 Nov 2011 at 2:28

GoogleCodeExporter commented 9 years ago
BTW, I really think I prefer "internal" over "impl".  Impl implies 
implementation classes for interfaces, as opposed to internal which implies 
inner workings.

Original comment by kandpwel...@gmail.com on 22 Nov 2011 at 2:42

GoogleCodeExporter commented 9 years ago
+1 on 'internal'... I was thinking the same thing after writing.

Original comment by codeto...@gmail.com on 22 Nov 2011 at 4:06

GoogleCodeExporter commented 9 years ago

Original comment by kandpwel...@gmail.com on 23 Nov 2011 at 1:55

GoogleCodeExporter commented 9 years ago
Submitted and ready for review

Original comment by codeto...@gmail.com on 24 Nov 2011 at 11:41

GoogleCodeExporter commented 9 years ago
setting status for code review 

Original comment by codeto...@gmail.com on 24 Nov 2011 at 11:57

GoogleCodeExporter commented 9 years ago
not ready to close this yet.  I was wondering about flattening it a bit more, 
and hiding more.  Per your original suggestion, I was thinking everything that 
is currently in package org.funcito.stub would go in org.funcito.internal.stub 
(and hence the ...internal packages within cglib and javassist get flattened).  
Thoughts?

Original comment by kandpwel...@gmail.com on 7 Dec 2011 at 5:06

GoogleCodeExporter commented 9 years ago
Also, I was thinking of separate public API classes in separate packages for 
each of the target libraries, because eventually there could be static 
namespace collision (multiple "functionFor()"s or such). I think you had 
suggested this earlier.  So:
org.funcito.guava.Funcito (or FuncitoGuava?)
org.funcito.fj.Funcito (or FuncitoFJ?)
org.funcito.java.Funcito (or FuncitoJava?)
org.funcito.jedi.Funcito (or FuncitoJedi?)

Original comment by kandpwel...@gmail.com on 7 Dec 2011 at 2:18

GoogleCodeExporter commented 9 years ago
This suggestion (and acknowledgement) renews my faith in humanity.

I prefer FuncitoX for values of X.

Original comment by codeto...@gmail.com on 7 Dec 2011 at 2:25

GoogleCodeExporter commented 9 years ago
I'm glad you still consider me to be a part of humanity, despite the fact that 
I like tabs.  What about comment #9, i.e., the flattening part.

Original comment by kandpwel...@gmail.com on 7 Dec 2011 at 6:14

GoogleCodeExporter commented 9 years ago
I forgot about the tabs. Please retract Comment 11.

I like org.functio.stub going into org.funcito.internal.stub but not sure I 
understand 'flatten'. I strongly recommend keeping:

org.funcito.internal.stub.cglib
org.funcito.internal.stub.javassist

I don't see any value in dumping a mixed bag of classes in 
org.funcito.internal.stub, if that is the suggestion (?).

Original comment by codeto...@gmail.com on 7 Dec 2011 at 7:01

GoogleCodeExporter commented 9 years ago
We're on the same page... I didn't explain clearly originally.  Currently there 
are: org.funcito.internal
org.funcito.stub.cglib.internal
org.funcito.stub.javassist.internal

By flattening, I just meant the part about having one "internal" root 
(org.funcito.internal), but yes, still have .stub.cglib and .stub.javassist 
underneath

changing status back to accepted

Original comment by kandpwel...@gmail.com on 7 Dec 2011 at 7:14

GoogleCodeExporter commented 9 years ago
Roger copy. Sounds good to me.... btw, in Groovy flattening a list like [ [a,b] 
[c,d] ] results in [a,b,c,d]

Original comment by codeto...@gmail.com on 8 Dec 2011 at 1:46

GoogleCodeExporter commented 9 years ago
I have refactored the stub packages, and checked-in. No API objects at this 
time.

Original comment by codeto...@gmail.com on 9 Dec 2011 at 1:09

GoogleCodeExporter commented 9 years ago
I will probably finish the other part of this as my next priority (besides 
resolving the basic CloudBees issue).  Raising priority because it affects the 
basic API by changing the packaging.  So I would like to get this fixed in the 
initial release.

Original comment by kandpwel...@gmail.com on 12 Dec 2011 at 7:11

GoogleCodeExporter commented 9 years ago
adding milestone

Original comment by kandpwel...@gmail.com on 16 Dec 2011 at 6:06

GoogleCodeExporter commented 9 years ago
So here are some critical choices that I want to make, soliciting your input.

I will probably add to the Funcito class itself, the following static methods:

public static GuavaDelegate guava() { return Guava.delegate; }
public static FjDelegate guava() { return Fj.delegate; }
public static JediDelegate guava() { return Jedi.delegate; }

... in which case I would probably want to change "stub()" in each of them to 
"callsTo()" so that you could  just call:

guava().callsTo( guava.functionFor(My.class).someMethod() );

What I need to decide is whether or not to pull up the static nested classes 
[Guava, Fj, and Jedi] to org.funcito.Guava, etc...  Part of me likes one-stop 
shopping in the Funcito class (which you could still *kind of* do with the new 
static methods I am proposing above).  The other part of me says it is rather 
unconventional, since most likely folks will use "import static 
org.funcito.Funcito.Guava.*" which doesn't feel quite right to be constantly 
importing *only* nested classes.  So I think I am leaning more towards the 
former: pulling up the nested classes.

What I really want to lock down is the core API and packaging, because this is 
sort of the "lock-in" point.  It gets more painful to change after a first real 
release.  If you see anything else questionable, please let me know about those 
too.

Original comment by kandpwel...@gmail.com on 21 Dec 2011 at 5:15

GoogleCodeExporter commented 9 years ago
I am a bit behind on the changes that have been made, and am confused by the 
latest comment. It seems like there are multiple suggestions here.

My issues with the proposed static methods include (a) why are we exposing an 
internal delegate object to the client? (b) they are all named guava() (which 
is just a typo).

I don't understand what you mean by 'pulling up the nested classes' but I think 
I agree with that point.

It would be nice to see the following:

org.funcito.FuncitoGuava
org.funcito.FuncitoFj
org.funcito.FuncitoJedi

with the standard static methods. I guess that means that Funcito.java 
evaporates. To me, this is plain and simple from the client perspective.

Original comment by codeto...@gmail.com on 22 Dec 2011 at 2:33

GoogleCodeExporter commented 9 years ago
Yes, it was a typo to have guava() for all 3 static methods.

"Pull up" means to make them top-level classes rather than nested classes, as 
your example above demonstrated, so I think you got the gist.

My hold out on the Funcito.java (which would only have the above 3 static 
methods and any other APIs that get added in the future) is so that folks have 
the option of making calls like:

guava().functionFor( guava.callTo(My.class).someMethod() );

You wouldn't use it to declare and visibly instantiate a delegate.  So the 
Funcito class would be diminished in significance, but would serve as an 
optional single entry point to the API.  Users could of course just static 
import FuncitoGuava.* -- that's what I would do when I use this.  User docs and 
Javadoc would clearly describe the two options.

Anyway, I am *not* dead-set on having the Funcito class and these methods.  So 
if you want to make any further case against them, I am all ears.

Thanks for the feedback, I will at least pull-up the FuncitoGuava, FuncitoFJ, 
and FuncitoJedi classes, and optionally make those static methods.

Original comment by kandpwel...@gmail.com on 22 Dec 2011 at 3:42

GoogleCodeExporter commented 9 years ago
I have pulled up the nested classes.  For the moment, I have left Funcito.java 
in, and added the 3 static methods I described.  Still open to removing these, 
but I needed to do something one way or another.

Original comment by kandpwel...@gmail.com on 23 Dec 2011 at 6:14

GoogleCodeExporter commented 9 years ago
Since I'm seeing no comments, I am changing the status to code complete.  For 
examples of possible usage of the Funcito.guava(), etc. static methods see 
Javadoc

Original comment by kandpwel...@gmail.com on 27 Dec 2011 at 8:58

GoogleCodeExporter commented 9 years ago
Done

Original comment by kandpwel...@gmail.com on 30 Dec 2011 at 5:15