mrszj / google-guice

Automatically exported from code.google.com/p/google-guice
Apache License 2.0
0 stars 0 forks source link

Followup matters to r1185 #526

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
In r1185 my idea of moving general utility classes to the package 
com.google.guice.internal.util to render the main internal package more 
comprehensible to people approaching the codebase was accepted.

I have two followup points:

1) Despite being mentioned in the list of files in the commit message,
Function.java was not actually moved. I think it ought to be.

2) Does anyone feel that having the util package depend on _nothing_ in
the main internal package (other than @Nullable, which can't really be
helped) is a good thing to strive for? If so, I shall submit a patch
that that moves parts of c.g.i.i.MoreTypes to c.g.i.i.util.Classes to break the 
last non-annotation dependency of the c.g.i.i.util package onto the c.g.i.i 
package.
and are used within the util package into util.Classes.

Original issue reported on code.google.com by m...@j.maxb.eu on 21 Jul 2010 at 1:01

GoogleCodeExporter commented 9 years ago
re 1) I moved Function in r1197

re 2) I don't see a any c.g.i.i dependencies in c.g.i.i.util.Classes.  After 
moving Function, the only dependencies on c.g.i.i that I see in c.g.i.i.util 
are in StackTraceElements & LineNumbers.  The former calling 
MoreTypes.memberType & the latter calling MoreTypes.memberKey (and the only 
caller of it).  I fixed LineNumbers in r1198 (for reasons written in the commit 
message).  The only way to fix StackTraceElements would be to replicate the 
functionality, though, and I don't think the issue is so important as to cause 
code duplication.

Original comment by sberlin on 1 Aug 2010 at 7:40

GoogleCodeExporter commented 9 years ago
Thanks, you've applied most of my suggestions.

However, the dependency of c.g.i.i.util.StackTraceElements on c.g.i.i.MoreTypes 
remains.

My suggestion (which I didn't explain very clearly, sorry) is to move 
MoreTypes.memberType and MoreTypes.toString to c.g.i.i.util.Classes.

Not only does this break the last (non-annotation) dependency of the 
internal.util package on the main package, it also neatly removes from 
MoreTypes these two methods which have nothing to do with generic Types and the 
rest of the MoreTypes class.

Not essential, I'll agree, but it does neatly finalize the demarcation between 
internal and internal.util.

Original comment by m...@j.maxb.eu on 2 Aug 2010 at 11:20