+1 to fixing compiler warnings. I would prefer proper usage of the wildcard type to just suppressing the warnings. I think writing PCollection<?> just means a PCollection parameterized on any type which holds the same meaning as PCollection, but PCollection<?>, to me, makes it clearer that PCollection is not a concrete type.
Robert On Mon, Jun 18, 2012 at 7:57 AM, Josh Wills <[email protected]> wrote: > On Mon, Jun 18, 2012 at 6:24 AM, Gabriel Reid <[email protected]> > wrote: > > As prep for some other development I was going to do in Crunch, I was > > considering cleaning up some (or all) of the compiler warnings that > > are currently occurring (they make the right-side search ribbon in > > Eclipse almost unusable right now). > > > > A significant portion of the compiler warnings are raw type generics > > warnings, i.e. "xxx is a raw type. References to xxx should be > > parameterized", where we're doing general operations with PCollections > > and similar objects without knowing anything about their generic > > types. > > There are also the warnings about not adding serialization UIDs to the > built-in DoFns, which irritate me and are useless in the context of > Crunch. Happy to volunteer to go around and add UID = 1; code all over > the place if there are no objections. > > > > > We could add wildcards (i.e. PCollection<?>) to each of these generic > > objects in the methods where the warnings are occurring -- this would > > be my preferred thing to do. On the other hand, I think that adding > > wildcards make the code more difficult to read, while having any kind > > of real added value. > > > > The other option we could take (less preferable to me) is to use > > @SuppressWarnings("rawtypes") on some or all of the affected methods > > -- it'll leave the code in a more readable state, but I'm not that > > wild about just suppressing warnings. > > I'm a 0 on the approach here-- I always have a hard time getting the > <?> stuff to compile when I'm casting the result, which is what often > happens in Writables.java and Avros.java, but I agree that a working > version of the wildcards is preferable to suppress warnings. We might > say that we prefer <?> but add in SuppressWarnings when there is no > other option for what we're trying to do. > > > > > Anyone else care to weigh in on this? > > > > - Gabriel > > > > -- > Director of Data Science > Cloudera > Twitter: @josh_wills >
