How many warnings is too many?

Discussion in 'Java' started by Rhino, Dec 13, 2005.

  1. Rhino

    Rhino Guest

    I suppose this is a rather odd question but how many warnings is too many
    when you are using a Java compiler like the one in Eclipse 3.1.1?

    Using the default settings for the 1.5 compiler in a fresh install of
    Eclipse 3.1.1, my various projects have over 4000 messages between them, 22
    of which are 'errors' that I have yet to repair. The rest are warnings of
    one kind or another. I could decrease this number substantially by either
    setting many conditions to 'error' or 'warning' when they are currently
    'ignore' or increase it substantially by setting 'ignore' conditions to
    'error' or 'warning'. And, of course, I could probably make the vast
    majority of them go away simply by rewriting code.

    I'd be surprised if anyone turned _all_ conditions to 'warning' or 'error'
    and then fixed every single one of the warnings and errors so that the
    entire workspace was error-free but maybe I'm wrong. That's why I'd like
    some feedback on just how many errors and warnings is considered okay and
    what is considered excessive.

    To put it another way, if you were considering hiring me to write Java for
    you and asked me for an example of a completed project to inspect, what
    conditions would YOUR compiler be set to detect and ignore and how many
    errors and warnings would you consider acceptable? I'm going to guess that
    the number of errors should always be zero in a completed project - correct
    me if I'm wrong - but that some warnings would be tolerated, on the theory
    that fixing every conceivable minor warning is not required to prove that
    you write good code. Am I close?

    Can anyone give me guidelines to help me determine just what warnings I
    should never tolerate and which I should feel free to ignore without making
    me look like a bad coder?
     
    Rhino, Dec 13, 2005
    #1
    1. Advertisements

  2. Rhino

    Adam Maass Guest

    Not quite. I would be more interested in probing a candidate's ability to
    fix the warnings and errors, and what the candidate had to say about why
    some of those warnings can be/should be ignored. In general, you should be
    able to write code that does not generate *any* warnings even with every
    conceivable flag turned on. In specific instances, some warnings are
    tolerable, but as a job candidate, you must be able to explain why.


    YMMV.


    -- Adam Maass
     
    Adam Maass, Dec 13, 2005
    #2
    1. Advertisements

  3. Working as a US gov't subcontractor I know that the Dept of Justice does
    not allow any code to be put into operations if it has even 1 warning as
    a result of a compile.
     
    Brandon McCombs, Dec 13, 2005
    #3
  4. Rhino

    Stefan Ram Guest

    This seems to encourage

    @SuppressWarnings("all")

    on the outermost elements of every compilation unit.
     
    Stefan Ram, Dec 13, 2005
    #4
  5. In my java projects, which are large and highly interdependent, I turn
    on all warnings. The only warning I have not yet turned on are:
    1. Usage of non-externalized strings
    2. Serializable class without serialVersionUID
    3. Boxing and unboxing conversions
    4. Missing @Overrride @Deprecated annotations
    5. Not all enum constants covered on 'switch'
    As time permits, I will turn on all. I have found many logic errors and
    latent bugs from the warning messages (that would have been off by
    default). It just makes for better code.
     
    Missaka Wijekoon, Dec 13, 2005
    #5
  6. Rhino

    Chris Smith Guest

    Prior to Java 1.5, I would have been kept up at night by warnings in
    pretty much anything. As of Java 1.5, it's unfortunately become a
    necessity that warnings exist because of type erasure with generics --
    it's really not possible to do many things with generics without
    warnings. Nevertheless, I try hard to minimize them.

    I do, though, turn some potential warnings off in Eclipse. This
    includes things that just don't matter to me most of the time (for
    example, missing serialVersionUID in a Serializable class); things that
    aren't even potentially problematic and are in fact documented and
    widely used features (accessing private members of enclosing types;
    declaring an unnecessary checked exception); and specialized "Eclipse-
    isms" (non-externalized strings).

    --
    www.designacourse.com
    The Easiest Way To Train Anyone... Anywhere.

    Chris Smith - Lead Software Developer/Technical Trainer
    MindIQ Corporation
     
    Chris Smith, Dec 13, 2005
    #6
  7. Rhino

    Chris Uppal Guest

    "Warning: line 33: getColor(), localised spelling detected in identifier"

    (Totally imaginary example, but it serves to illustrate the idiocy of requiring
    "no warnings" when the set of conditions the compiler sees fit to warn about is
    not under the developer's -- or customer's -- control)

    -- chris
     
    Chris Uppal, Dec 13, 2005
    #7
  8. Rhino

    Chris Uppal Guest

    I believe that many people's impressions of compiler warnings are conditioned
    by their experience (not necessarily direct) with C and its compilers. There,
    the only safety comes from reading compiler "warnings", understanding them, and
    taking them seriously (if not necessarily changing the code). That is to say,
    they warn of potential errors -- a language with a different philosophy would
    treat them as actual errors.

    Java is not like that; unless the compiler can convince itself that (within
    certain limits) the code is OK, it will refuse to compile it. The "warnings"
    don't alert you to cases where it is unable to do those checks (e.g.

    "Waning: line 44: array indexing operation, bounds not checked"

    ) but where some programmer has decided that "his" idea of what good code looks
    like is better informed than "yours". I don't -- particularly -- object to the
    compiler issuing warnings (in fact there are a number of cases where I wish it
    would warn more), but it seems to me that some people display an overly
    respectful attitude to compiler warnings.

    That said, I don't like my code to issue any warnings either. But that's
    because (a) I have a tidy mind ("Extremism, officer, is the sign of a tidy
    mind" -- some movie) and (b) I don't want to have to pick out any useful
    warnings from a blizzard of drivel.

    It's unfortunate that Eclipse muddles its built-in style checker with its
    compiler. That compounds the snow-storm effect. I'm more likely to turn an
    Eclipse warning off than change my code.

    Taking this perhaps more literally than you meant. I wouldn't care much about
    the number of warnings which showed up with his/her code in /my/ environment.
    The two questions I'd want answered would be:
    Are /any/ of the warnings evidence that the candidate does not understand
    Java ?
    And, as a question to put directly to the candidate: if their environment
    showed no warnings):
    How do you decide which warnings to turn off ?
    or if their environment showed any warnings (which I would take as a negative
    point anyway -- I like a tidy mind):
    How can you tell that there's nothing important in that list ?

    -- chris
     
    Chris Uppal, Dec 13, 2005
    #8
  9. I turn on most of the Eclipse warnings, but also have a separate
    CheckStyle configuration, which I invoke every so often from ant. It
    does a frankly pedantic series of checks, which would get in the way of
    the 30 or so persistent warnings about serialisation UIDs and
    deprecated API usages that I'm slowly removing.

    I'm removing the serialisation UID warnings simply by not subclassing.

    When I've no warnings left in Eclipse I might make Eclipse use my
    CheckStyle configuration to display warnings all the time. I find less
    than 50 warnings workable - otherwise it's difficult to see new
    warnings. I don't think Eclipse keeps a log of when the warning was
    first thrown up by the build.
     
    ricky.clarkson, Dec 13, 2005
    #9
  10. Rhino schreef:
    I use warnings as a TODO marker for more serious things: if there is no
    javadoc for a method, that means the method is not ok. But I suppose
    that´s not really a good idea.

    I have turned on a lot of warnings, and got rid of them, from time to
    time using @SuppressWarnings, if I really need to clone an ArrayList,
    for example. OTOH, I could do return new
    ArrayList<Something>(oldArrayList), but I don´t really like that.

    Then, if you think you can use even more warnings, have a look at PMD:
    http://pmd.sourceforge.net/. Fortunately, you can have those warnings
    in a separate view in Eclipse.

    H.

    --
    Hendrik Maryns

    ==================
    www.lieverleven.be
    http://aouw.org
     
    Hendrik Maryns, Dec 13, 2005
    #10
  11. Rhino

    Chris Smith Guest

    That's the problem. It doesn't always make for better code. Here are
    just a couple examples:

    - The reasonable reader assumes that when a class contains a field
    called serialVersionUID, it's going to be serialized. Yet you can
    accidentally inherit the Serializable interface from all manner of
    places (by creating any custom AWT/Swing component, for example). If
    the class doesn't intend to be serialized, adding a serialVersionUID is
    worse than useless; it's confusing.

    - Access to private members of an enclosing class is a language feature
    that's used in powerful ways. Avoiding this just because Eclipse has
    the option to give a warning is passing up an opportunity to write
    cleaner code that keeps local things local. You end up being forced to
    break encapsulation so that your anonymous inner classes can do useful
    work. Again, worse than useless. The language rule that permits this
    access was written deliberately with the realization that object
    interactions don't always correspond to abstraction boundaries, and
    you're taking a step back by ignoring that important fact.

    - Boxing conversions, as well, are a language feature that's added for a
    reason. Sure there are gotchas there, but why anyone should aspire to
    be prevented from using boxing in a Java 1.5 project escapes me. Are we
    actively trying to make code hard to read?

    It makes very little sense to assume that the people who developed
    Eclipse somehow know better than anyone else what's bad for code. Many
    of their warnings are useful (for example, you ought to immeidately
    enable #4 and #5, IMO), but some are not.

    --
    www.designacourse.com
    The Easiest Way To Train Anyone... Anywhere.

    Chris Smith - Lead Software Developer/Technical Trainer
    MindIQ Corporation
     
    Chris Smith, Dec 13, 2005
    #11
  12. Rhino

    Roedy Green Guest

    On the other hand, if you inherit Serialized, are you not obligated to
    continue to support serialisation? Is there some convention to use to
    make sure your clients don't try?
     
    Roedy Green, Dec 13, 2005
    #12
  13. Rhino

    Roedy Green Guest

    An inner class without access to mother's fields and methods is just
    for all intents and purposes just an independent top level class. Why
    bother with an inner class if it has no outer references of any kind?
     
    Roedy Green, Dec 13, 2005
    #13
  14. Rhino

    Roedy Green Guest

    I think the notion here is to make people aware of the conversions.
    With autoboxing you could blithely be converting back and forth when
    you don't need to. An IDE could handle this with a bit of colour or a
    subtle glyph to mark the automatic conversions. I agree it is goofy
    to write the code out longhand. It just muddies the logic.
     
    Roedy Green, Dec 13, 2005
    #14
  15. Rhino

    Chris Smith Guest

    You are certainly contractually obligated to make serialization work in
    a well-defined manner. However, you're perfectly welcome to document
    that Serialization will not be compatible across releases, which is the
    problem that serialVersionUID solves. For example, Swing documents that
    restriction in all component classes (which inherit Serializable from
    java.awt.Component).

    However, if you do add a serialVersionUID, then you are VERY strongly
    obligated to make sure that serialization does work across releases of
    the class, because attempting to do so will no longer automatically
    fail. Adding that field means taking on a huge responsibility, and who
    wants Eclipse deciding for them that the responsibility is necessary?

    --
    www.designacourse.com
    The Easiest Way To Train Anyone... Anywhere.

    Chris Smith - Lead Software Developer/Technical Trainer
    MindIQ Corporation
     
    Chris Smith, Dec 13, 2005
    #15
  16. Rhino

    Chris Smith Guest

    If that's the point, they needed to add a feature to the syntax
    highlighter. I agree that that could be helpful (presuming that they
    have the good sense to make it optional).

    Adding a warning is NOT appropriate for making people aware of this.
    Warnings are issued and listed for the entire project (or even
    workspace); they should only contain information that is important to
    anyone who looks at ANY part of the project.

    --
    www.designacourse.com
    The Easiest Way To Train Anyone... Anywhere.

    Chris Smith - Lead Software Developer/Technical Trainer
    MindIQ Corporation
     
    Chris Smith, Dec 13, 2005
    #16
  17. Hrm. I think this must mean there's something wrong with the whole
    idea of serialVersionUID.

    I have a bunch of classes that extend JPanels and an assortment of
    other JComponents. They're not intended to be serialized. They're
    not intended to be used as an API. There is no good reason for anyone
    to be serializing them, or to be extending them and then griping that
    they aren't serializable.

    It's one thing if you're writing an API, even for internal use. But
    when you're just writing an app? Come on!
     
    Monique Y. Mudama, Dec 13, 2005
    #17
  18. Monique,

    There's no need to extend JPanel or JComponent. Most extensions are
    just a way of saving typing (as in keypresses). The one that *looks*
    like it's valid is overriding paintComponent for custom drawing, but
    even that can be better done using an Icon and, usually, a JLabel.

    If your class isn't supposed to be Serializable, but extends a class
    that is, then you have a logic error. You are subclassing something
    you shouldn't.

    It's like buying a car and then complaining when someone asks for a
    lift. Oh yeah, that happens..
     
    ricky.clarkson, Dec 14, 2005
    #18
  19. I don't like extending classes where it isn't necessary. For normal uses
    there is no point extending JPanel. But banning a component extending
    JComponent is absurd.

    Tom Hawtin
     
    Thomas Hawtin, Dec 14, 2005
    #19
  20. Roedy Green schreef:
    Don´t you think it could be meaningful if the (static) inner class is
    semantically very connected to its enclosing class? For example, I have
    a class Function, which has a method getDomain, which returns an element
    of the public static inner class Domain. Domain is nothing but a small
    container class for two set variables, with appropriate getters. I
    could very easily make that a top level class, but it sort of seems to
    say something about the nature of this class that it belongs to the
    Function class. Function.Domain is more meaningful than just Domain.

    Until of course I think of other uses for the Domain class, but that
    seems unlikely.

    I´m just asking for an opinion here, not criticising.

    H.
    --
    Hendrik Maryns

    ==================
    www.lieverleven.be
    http://aouw.org
     
    Hendrik Maryns, Dec 14, 2005
    #20
    1. Advertisements

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments (here). After that, you can post your question and our members will help you out.