IG: Concrete class for instance variable

Shows error if concrete class which does not implement any interfaces is
used for instance variable. I think this case should not be considered an
error(warning) or at least be configurable


5 comments
Comment actions Permalink

Alex Roytman wrote:

Shows error if concrete class which does not implement any interfaces is
used for instance variable. I think this case should not be considered an
error(warning) or at least be configurable

Agreed.
Dave?

--
Maxim Shafirov
IntelliJ Labs / JetBrains Inc.
http://www.intellij.com
"Develop with pleasure!"

0
Comment actions Permalink


The purpose of this family of inspections is to show places where you are creating dependencies on implementations, rather than interfaces. How about instead of ignoring implementations which have no interfaces, a filter for ignoring either or ? Would that fit the use case better?

--Dave Griffith

0
Comment actions Permalink

Dave,

This inspection only should say you have an interface why are you using
implementation (BTW I believe it should optionally trigger only if both
interface and implementation is in my project)

It should not say why not all your objects have interface tyo them. That
could be purpose of another inspection

In many cases you do not need interfaces for your objects. Being overzealous
might make this inspection less useful


"Dave Griffith" <dave.griffith@cnn.com> wrote in message
news:12052612.1085061284256.JavaMail.itn@is.intellij.net...
>

The purpose of this family of inspections is to show places where you are

creating dependencies on implementations, rather than interfaces. How about
instead of ignoring implementations which have no interfaces, a filter for
ignoring either or [implementations
from library jars which have no interfaces declared (with possible
exceptions for Serializable and Cloneable)]? Would that fit the use case
better?
>

--Dave Griffith



0
Comment actions Permalink

Alex Roytman wrote:

Dave,

This inspection only should say you have an interface why are you using
implementation (BTW I believe it should optionally trigger only if both
interface and implementation is in my project)

It should not say why not all your objects have interface tyo them. That
could be purpose of another inspection

In many cases you do not need interfaces for your objects. Being overzealous
might make this inspection less useful


"Dave Griffith" <dave.griffith@cnn.com> wrote in message
news:12052612.1085061284256.JavaMail.itn@is.intellij.net...

>>The purpose of this family of inspections is to show places where you are


creating dependencies on implementations, rather than interfaces. How about
instead of ignoring implementations which have no interfaces, a filter for
ignoring either or [implementations
from library jars which have no interfaces declared (with possible
exceptions for Serializable and Cloneable)]? Would that fit the use case
better?

>>--Dave Griffith

Here are some thoughts:
1. The visibility of the concrete class affects the importance of this
inspection. Certainly if the concrete class is private to the using
class, the inspection shouldn't trigger. If it is package private, then
it is much less dangerous than if it were a public class.
2. Alex, there are cases where you would want to be warned about a
concrete class even if it has no interfaces declared. Perhaps the class
should have an interface. I think this was Dave's original intention.
However, introducing an interface increases complexity, and the simplest
thing may be to just leave the design the way it is, so this warning
should be optional. I agree that it's a good idea to have two separate
inspections, one for "It has an interface, why aren't you using it?",
and one for "Maybe it should have an interface".

--
Rob Harwood
Software Developer
JetBrains Inc.
http://www.jetbrains.com
"Develop with pleasure!"

0
Comment actions Permalink


Got it. This is starting to seem more reasonable (and much more useful). Let me chew over some possible semantics a bit, and see if I can propose something that fits these use cases. I'm not sure that simply having an interface should be the trigger for "Why aren't you declaring this as an interface?", though. After all java.lang.String has an interface, but it would look very odd indeed if IG told you declare your String variables as java.lang.CharSequence instead. For the "Shouldn't this class expose it's functionality via an interface?" issue, there is a new inspection in build 2018 for public methods which are not exposed via an interface, which still might be too picky but is a start.

Tricky problem, and pretty valuable in terms of improving designs if we get the right answer...

--Dave Griffith

0

Please sign in to leave a comment.