commons-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Shevek (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (LANG-819) EnumUtils.generateBitVector needs a "? extends"
Date Fri, 10 Jan 2014 09:21:52 GMT

    [ https://issues.apache.org/jira/browse/LANG-819?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13867645#comment-13867645
] 

Shevek commented on LANG-819:
-----------------------------

I didn't explain that last point very well due to lack of patience. Hopefully this will help,
if you forgive the background:

There's an almost-requirement in C/C++ (auto_ptr aside) that a data structure have an "owner"
and a number of referrents. The same happens in Java as good style, rather than requirement-to-avoid-leak.
A complex class may own a Set<E>. The getter of this set may be typed Set<? extends
E>. This tells any other person who takes that reference that they should not (and can
not, due to compile failure) modify that set. On the other hand, if it's a factory method,
the method may be typed Set<E>. This tells the person who takes the reference that they
may (and can) modify that set. This generally comes with the assumption that unsynchronized
mutation of the set is also permitted, modifying it will not break the encapsulation of the
provider, and it isn't shared with any other threads. Yes, there are loopholes in this, but
it's pretty good, vastly improves the ability of the compiler to show code correct, and reduces
programmer mistakes (unnecessary copies, accidental modifications, etc).

It isn't really a surprise to anyone who has coded in Erlang or any monad-based language that
it's actually relatively easy to write Java code where the majority of structures are never
mutated after construction. Some languages have this as a restriction, but it turns out to
be a fairly good idea even in Java: Most sensible ways to pass values between threads create
a happen-before relationship in the JMM, and remembering that final variables have a happen-before
relationship with the end of a constructor (something explicitly NOT true for nonfinal variables),
this also makes it much easier to write highly threaded code correctly.  The vast majority
of instance variables can be final, and modern IDEs tend to provide this as a default hint
if they detect non-mutation, so this really is hitting mainstream. By this set of practices
we get a applications where the majority of Collections are typed ? extends, the majority
of variables are final, etc, etc.

Anyway, this is all a suite of techniques for creating compile-time checked, reliable, threaded,
idiot-developer-safe Java code.

At this point, it isn't about whether EnumUtils will modify the set. I know it won't. It's
the fact that the code that CALLED EnumUtils only ever had a Collection<? extends E>
in the first place, because the _actual_ owner of the collection, several calls back, decided
to export it only as readonly.

The Guava library (which I mostly use these days) is very well written with respect to all
these techniques, and applies them universally. See http://docs.guava-libraries.googlecode.com/git/javadoc/index.html?com/google/common/collect/Iterables.html
- which shows how Guava handles iterables. The type within the Iterable is irrelevant. The
point is, the caller of Iterables (and even back several frames from there) only had a Set<?
extends> or a Collection<? extends> and the CALLER would have to upcast it in order
to call EnumUtils with the current prototype.

So, you see, it isn't about a tiny religious issue about whether I like the current prototype
or not. It's that in any code written with these good coding practices, I CAN'T call EnumUtils
without upcasting the Set<? extends E> to a Set<E> in the previous frame, and
if I upcast, my code won't pass review. If I use a raw type, my code won't pass review. There
is NO WAY in which I can call this function and get code past review. I CAN NOT USE MUCH OF
COMMONS. Guava, on the other hand, uses ? extends in every case where it's legal to do so,
and can always be called from well-written type-safe code, and has consequently become the
go-to library of choice. (That's also why this ticket languished for a year: we also ported
away from commons to guava. But I did come back to try to help.)

Bootnote on Guava: It's Google's policy not to include any routine for which it does not have
an internal customer. Apache does not have the same policy. In a recent experience, I submitted
correct code and patches to Apache, with test suites, only to have the Apache authors fix
up the original broken code to (just) pass the cases in my test suites, but still contain
bugs because they fundamentally aren't internal customers for it and don't use it in anger
or in generality. So it remains broken in Apache. My last experience with that was type (reflection)
manipulation routines - my job is writing compilers, so I'm very hot on them, and familiarity
breeds ease of spotting bugs, especially in type systems. But I won't play whack-a-mole submitting
test suites to ASF when I have working code anyway. I'll just use my code. So there's some
additional background to my frustration with ASF maintainers. While Google publish smaller
libraries, the quality is unquestionably much higher because of this policy of (colloquially)
"If we can't dogfood it, we won't even publish it." They are also 100% familiar with the techniques
I described in this comment, and reading any of their libraries in C++ or Java is an education
in high-performance compile-time-checked coding. Of course what they don't do is put JSR305
annotations on their public APIs. I wish they did. I wish Apache would use JSR305, too.

> EnumUtils.generateBitVector needs a "? extends"
> -----------------------------------------------
>
>                 Key: LANG-819
>                 URL: https://issues.apache.org/jira/browse/LANG-819
>             Project: Commons Lang
>          Issue Type: Bug
>          Components: lang.*
>    Affects Versions: 3.0.1
>            Reporter: Shevek
>            Priority: Minor
>
>     public static <E extends Enum<E>> long generateBitVector(Class<E>
enumClass, Iterable<E> values) {
> Should be Iterable<? extends E>.
> This is because although no subclasses of E can exist, the "? extends" is a common idiom
for marking the collection as readonly, or not "owned" by the current object.



--
This message was sent by Atlassian JIRA
(v6.1.5#6160)

Mime
View raw message