@Suppress this



  • Our overlord auditors asked me to do a code-review of a system written by another team. Upon digging in, I noticed that every method and function had @SuppressWarnings("{a,b,...}") in front of it. Curious, I stream edited the entire tree to remove all those lines, and then compiled it. Over 56,000 warnings!

    Why did they do this? Compliance had issued a directive that the quality of all code was to be improved for the purpose of stability, and one part of this was to write the code such that there were no compiler warnings. Accordingly, the developers of the other team were told to fix the code so that there were no compiler warnings!

    While their solution did technically result in no compiler warnings, the auditors to whom I had to make my report were not amused.

    At the moment, there is a flurry of "you f***'d up" emails going back and forth.

    The team manager is trying to defend their solution because doing what the compliance folks wanted would take too much time, and would add no value to the application.

    The compliance folks are saying that the whole point was to improve production stability, and that all project managers were informed beforehand that they would be given the required time to make these changes.

    Guess who's going to lose this one.

    Aside: another team audited our turd-of-an-app. As I'd gone over it with a fine-toothed comb after the last release, it was mostly clean, so they didn't find much. I fixed all the glitches so by the time the auditors got it, we were gold. Ironically, the app still sucks, runs like a crippled snail stuck in molasses on a cold winters' day, and is architecturally a bowl of spaghetti, but we got our gold star!

     



  • Hmm. SUppressing warning can be a valid thing to do on a blanket basis. The reason being that you can then establish a 'Day zero', after which compilation must be warning free - this will catch any new violations.

    Couple this with an ongoing program of steadily removing the old supressions and fixing the underlying code to ensure no wranings are produced, and you can get to where you want to be.

    Having 56k warnings can be problematic - where the hell do you start?



  •  It is likely that the warnings are similar to the W3C validator's warnings, in that modifying a single line of code removes half of the warnings outright.



  • These are more along the lines of uninitialized variables, using Object instead of an appropriate type, and so on. Mostly lazy coding nonsense and usually easily fixed.

    When I did it for our system, I removed all the suppression, started with the first warning and kept going until the IDE's warning list was empty. After the first few you start to see repeats, so it goes fairly quickly. In my case, it took me a solid week to clean it up.



  • @snoofle said:

    Curious, I stream edited the entire tree to remove all those lines, and then compiled it. Over 56,000 warnings!

    What did you use to do this?

     



  • @zelmak said:

    @snoofle said:

    Curious, I stream edited the entire tree to remove all those lines, and then compiled it. Over 56,000 warnings!

    What did you use to do this?

     

     

    for fn in `find . -name "*.c" -print`; do sed -i '/@SuppressWarnings/d' "$fn"; done

     



  • @nonpartisan said:

    @zelmak said:

    @snoofle said:

    Curious, I stream edited the entire tree to remove all those lines, and then compiled it. Over 56,000 warnings!

    What did you use to do this?

     

     

    for fn in `find . -name "*.c" -print`; do sed -i '/@SuppressWarnings/d' "$fn"; done

     

     

    Crap.  Too quick on the draw.  Should've been "*.java".  I need a holiday.

     



  • @nonpartison - pretty much exactly that



  • @snoofle said:

    @nonpartison - pretty much exactly that

    Got lucky with none of them being spread over multiple lines, had to fix them up manually, or used a multi-line matching regex?




  • @DaveK said:

    @snoofle said:

    @nonpartison - pretty much exactly that

    Got lucky with none of them being spread over multiple lines, had to fix them up manually, or used a multi-line matching regex?


     

    I figured if there was much stuff that crossed line boundaries I could always wipe it, check it out again and figure out something else. As it happens, there was only one annotation that spanned multiple lines, so I just fixed it by hand.



  • @snoofle said:

    Guess who's going to lose this one.

    Ooo, ooo, I know! Snoofle.



  • @blakeyrat said:

    @snoofle said:
    Guess who's going to lose this one.
    Ooo, ooo, I know! Snoofle.
    Exactly what I thought, too.



  • I totally sympathize with the guys hiding the warnings. 

     

    My cool story, bro:

    A large chunk of our application runs on IronPython. That's a WTF in and of itself, but after numerous production performance issues, our main performance guy included the IronPython source in our solution.  So, being the paragon of good code that IronPython is, it throws a lot of warnings when building.  Getting rid of warnings is great and all, but our team doesn't really have the bandwidth to clean up IronPython.  So I suppressed the warnings.



  • @Scarlet Manuka said:

    @blakeyrat said:
    @snoofle said:
    Guess who's going to lose this one.
    Ooo, ooo, I know! Snoofle.
    Exactly what I thought, too.

    Dammit guys you spoiled the punchline! :(



  • @vt_mruhlin said:

    I totally sympathize with the guys hiding the warnings. My cool story, bro:<snipped>
    I would sympathize too, however in snoofles case, they didn't include the java core library into their solution...  All those warnings were from shite code written by shite developers.  I have no patience or sympathy for apathy and laziness.  And that BS about "they just didn't know any better" is just a ruse for lazy and apathetic developers who don't want to better themselves.

    Everybody has seen the type, where they create code like this:

    if (myBaseObject.GetType().ToString().ToLower().Trim() == myOtherObject.GetType().ToString().ToLower().Trim())

    and never once do they think to themselves, "there MUST be a better way"...

    /rant



  • @C-Octothorpe said:

    @vt_mruhlin said:

    I totally sympathize with the guys hiding the warnings. My cool story, bro:<snipped>
    I would sympathize too, however in snoofles case, they didn't include the java core library into their solution...  All those warnings were from shite code written by shite developers.  I have no patience or sympathy for apathy and laziness.  And that BS about "they just didn't know any better" is just a ruse for lazy and apathetic developers who don't want to better themselves.

    Everybody has seen the type, where they create code like this:

    if (myBaseObject.GetType().ToString().ToLower().Trim() == myOtherObject.GetType().ToString().ToLower().Trim())

    and never once do they think to themselves, "there MUST be a better way"...

    /rant

    Yeah, good programming is a lost art, teh codez should be like this:

    if (myBaseObject.GetType().ToString().ToLower().Trim().Equals( myOtherObject.GetType().ToString().ToLower().Trim()))

    I mean, is not that hard isn't it? ;)


  • BINNED

    @vt_mruhlin said:

    A large chunk of our application runs on IronPython. That's a WTF in and of itself

    Could you elaborate on that? I'm curious.
    I think Python is a pretty nice language, though I've only used it on toy projects and never professionally. IronPython sounds like a nice idea, compiling it bytecode for a fast VM, but I've never used it. What's wrong with it?

    @vt_mruhlin said:

    included the IronPython source in our solution.

    What the hell? Why would you do that if you're not including patches to it.

    I'm imagining this happen for our app. It already takes about 45min to clean build on my i7 Quad Core. Adding the Qt library we use would probably blow that up by another 2-3 hours. Adding no benefit at all.

     

     

     



  • @serguey123 said:

    @C-Octothorpe said:

    @vt_mruhlin said:

    I totally sympathize with the guys hiding the warnings. My cool story, bro:<snipped>
    I would sympathize too, however in snoofles case, they didn't include the java core library into their solution...  All those warnings were from shite code written by shite developers.  I have no patience or sympathy for apathy and laziness.  And that BS about "they just didn't know any better" is just a ruse for lazy and apathetic developers who don't want to better themselves.

    Everybody has seen the type, where they create code like this:

    if (myBaseObject.GetType().ToString().ToLower().Trim() == myOtherObject.GetType().ToString().ToLower().Trim())

    and never once do they think to themselves, "there MUST be a better way"...

    /rant

    Yeah, good programming is a lost art, teh codez should be like this:

    if (myBaseObject.GetType().ToString().ToLower().Trim().Equals( myOtherObject.GetType().ToString().ToLower().Trim()))

    I mean, is not that hard isn't it? ;)

    I see how to fix this!

    if (myBaseObject.GetType().ToString().Trim().EqualsIgnoreCase( myOtherObject.GetType().ToString().Trim()))


Log in to reply