Regarding...

*> public interface A is returning Region and not Region<K,V> is that an
acceptable suppression?*

I think Geode API/Framework (e.g. PDX) (production) code should be very
explicit and *not* use *rawtypes*, or even *wildcard* types, for that
matter, as far as possible.

To be clear, I am saying sometimes in tests, *rawtypes* are useful.

Again, all of these things should be applied responsibly and judiciously,
with intent, especially in production code.

Use of *Java Generics* in code can cause as much grief for a user as it
does help in other cases (as Jake alluded to), so it needs to be careful
crafted, especially in public APIs as it affects more than test code, but
even application code that might require references to Geode objects.

-j


On Fri, May 8, 2020 at 1:30 PM Mark Hanson <mhan...@pivotal.io> wrote:

> It is slightly different, but here is a case I recently found.
>
> List<VersionTag> versionTags = versions.getVersionTags();
>
> This method is internal though.
> public List<VersionTag> getVersionTags() {
>   return Collections.unmodifiableList(this.versionTags);
> }
>
> Thanks,
> Mark
>
>
> > On May 8, 2020, at 1:25 PM, Mark Hanson <mhan...@pivotal.io> wrote:
> >
> > How does everyone feel about situations involving raw types?
> >
> > Such as public interface A is returning Region and not Region<K,V> is
> that an acceptable suppression?
> >
> > Thanks,
> > Mark
> >
> >> On May 8, 2020, at 1:20 PM, John Blum <jb...@pivotal.io> wrote:
> >>
> >> Agreed, but the following (inside tests) does not work in all cases,
> i.e.
> >>
> >> Region<String, String> region...
> >>
> >> Particularly if "region" is passed to a method with a different type
> >> signature.
> >>
> >> I am trying to find/think of the situation I encounter from time to
> time,
> >> even when I use the *wildcard* signature (i.e. Region<?, ?>), but cannot
> >> currently find it (*#ugh*).
> >>
> >> Anyway, the point is, if the test is really not concerned with the type
> of
> >> values (*keys*, *values*) being stored in the mock *Region* then
> rawtypes (or
> >> sometimes Region<?, ?>) are useful in some cases since the point of the
> >> test is not about the data but perhaps about *Region* configuration in
> >> general, so adding types detracts (adds undue noise) to the code under
> test
> >> (IMO).
> >>
> >> It depends and is subjective.
> >>
> >> I agree, though, in general, @SuppressWarnings should be kept to a
> minimum
> >> and used only when necessary.
> >>
> >> -j
> >>
> >> On Fri, May 8, 2020 at 1:09 PM Kirk Lund <kl...@apache.org> wrote:
> >>
> >>> Actually there is an alternative to using @SuppressWarnings for Mockito
> >>> mocks:
> >>>
> >>> Region<String, String> region = cast(mock(Region.class));
> >>>
> >>> private static <T> T cast(Object object) {
> >>> return (T) object;
> >>> }
> >>>
> >>> Personally, I'd prefer to see warnings or workarounds like above than
> see
> >>> lots of @SuppressWarnings littered throughout our code base. I think
> it's a
> >>> smell of bad code.
> >>>
> >>> On Fri, May 8, 2020 at 12:44 PM Jacob Barrett <jbarr...@pivotal.io>
> wrote:
> >>>
> >>>>
> >>>>
> >>>>> On May 8, 2020, at 12:41 PM, Donal Evans <doev...@pivotal.io> wrote:
> >>>>>
> >>>>> Is there a consensus on what constitutes a benign warning? I think
> the
> >>>> only
> >>>>> times I use @SuppressWarnings is in parameterized tests to suppress
> the
> >>>>> unused method warning for the parameters method, or for unchecked
> >>> casts,
> >>>> as
> >>>>> below:
> >>>>>
> >>>>> PartitionedRegion detailRegion1 = mock(PartitionedRegion.class);
> >>>>> when(cache.getRegion(regionPath1)).thenReturn(detailRegion1);
> >>>>>
> >>>>> where the thenReturn() would complain, since it's expecting to
> return a
> >>>>> Region<Object, Object>.
> >>>>>
> >>>>> Would these be considered things that could safely just be ignored
> (and
> >>>> so
> >>>>> for which I can turn off inspection), or is the use of
> >>> @SuppressWarnings
> >>>>> here warranted?
> >>>>
> >>>> This is a legitimate suppression. There is no other way to correct the
> >>>> dysfunctional nature of Java Generics than to @SuppressWarnings. In
> this
> >>>> case though only that statement should be suppressed.
> >>>>
> >>>> -Jake
> >>>>
> >>>>
> >>>
> >>
> >>
> >> --
> >> -John
> >> Spring Data Team
> >
>
>

-- 
-John
Spring Data Team

Reply via email to