0

Given that the concept of "best" and "worst" practices can be different in different environments, our company introduced a standard set of internal "best" practices that are continuously violated by our developers. I want to share two in particular:

  • Never handle exceptions with ex.printStackTrace(), instead log them
  • Never use System.out in a web application for debugging purposes, better use debug log

These two are fairly simple to detect as a simple Search command can display each violation.

I'd like to ask if anything can be done to prevent the coder from using these prohibited methods in his projects. Forcefully deprecating the System.out object or the printStackTrace method from out of the standard JVM would be a great idea as provides evidence of the mistake in the form of a warning.

Since we don't implement a human code review mechanism (code is reviewed only when issue arise, or a new developer takes charge of a module), I'm seeking for automated ways of preventing such common mistakes, including breaking the compilation itself. I'm thinking about something like a Java plugin in which I can configure special rules for what that is not an error in Java syntax to be considered as an error. ReSharper plugin for Visual Studio, for example, does a similar thing when dealing with naming and coding conventions.

usr-local-ΕΨΗΕΛΩΝ
  • 26,101
  • 30
  • 154
  • 305
  • 4
    `System.setOut(null); System.setErr(null);` - for the truly evil developer. – user253751 Dec 21 '14 at 11:07
  • 1
    Do you require code reviews before commit/check-in/merge/whatever-source-control-operation-you-use-when-something-is-done? – user253751 Dec 21 '14 at 11:10
  • @immibis I updated my question to clarify we don't have human code review sessions – usr-local-ΕΨΗΕΛΩΝ Dec 21 '14 at 11:15
  • This isn't a technical question. Or rather, the *real* question is: why do your developers not do what you think they obviously ought to do? Hemming them in with automated constraints isn't fixing the root cause. – DNA Dec 21 '14 at 11:18
  • 1
    You could automatically check your source code, e.g. PMD supports own rules: http://pmd.sourceforge.net/pmd-5.2.2/customizing/howtowritearule.html – Behe Dec 21 '14 at 11:18
  • 1
    @immibis `System.setOut(null);` would be likely to damage logging through a logging library if the logging library was configured to write to standard out. – Nicholas Daley-Okoye Dec 21 '14 at 11:20
  • 2
    @DNA I politely disagree. If there is a rule about how things should be done, then there should be tools or processes in place to check them. Ideally tools, so that it doesn't waste human time. Otherwise rules can be forgotten, or new people on a project might not learn the rule. And of course, it's easy for people in a hurry to do something wrong. – Nicholas Daley-Okoye Dec 21 '14 at 11:27
  • @Nicholas Daley In general I would agree (and I use such tools myself), but the phrase "continuously violated" suggests a deeper problem than merely forgetting a rule occasionally; e.g. that devs don't understand why these rules are there. – DNA Dec 21 '14 at 11:37
  • I might have misused the word *continuously*. Our situation is best described by @NicholasDaley (new people not yet learned the rule, people in hurry violate the rule and forget, copy&paste coding --> **new people learn from wrong examples**). Unfortunately we are now in the situation to need tools to detect old unfixed errors, instruct coders, and prevent future errors by reminding them. But the two are the same to me. I found a proverb for that: "you can't teach an old dog new tricks" – usr-local-ΕΨΗΕΛΩΝ Dec 21 '14 at 11:57

5 Answers5

1

You can use PMD and especially the rule AvoidPrintStackTrace to solve your specific problem.

PMD can be configured with your Sonar or Jenkins together with a bunch of other good rules/restrictions.

wassgren
  • 18,651
  • 6
  • 63
  • 77
1

The rules you have provided well known good Java coding practices. There are already available tools that can check your code and make sure these rules are not violated.

PMD has the rules you are looking for:

  • Never handle exceptions with ex.printStackTrace(), instead log them - AvoidPrintStackTrace

  • Never use System.out in a web application for debugging purposes, better use debug log - SystemPrintln

Adam Siemion
  • 15,569
  • 7
  • 58
  • 92
  • I'm happy to see me and my boss are not the only ones. Using PMD as code review helper seems feasible. I'll soon check its full capabilities according with our other coding practices – usr-local-ΕΨΗΕΛΩΝ Dec 21 '14 at 11:32
1

There are various style and bug checkers that can be tailored to detect custom style violations. Some can even be integrated with your company's preferred IDE.

But that doesn't solve your problem. If people are wilfully ignoring style rules, then politely asking them to run a style checker from their IDE isn't going to work.

I can think of two approaches that may work for you:

  • Set up a continuous integration (CI) server, configure Sonar analysis and reporting, and include your style checks.

  • Configure your version control system to run the style checker as a check-in hook. Get it to reject check-ins that increase the number of style violations.

The first approach can still be ignored by recalcitrant developers, but the evidence of the violation of company standards will be plain to see.

The second approach borders on "fascist" ... but if that is what it takes to get people to toe the line, then all you need is rock solid management support and a thick skin :-)

The other way to look at this is that this is a problem for developer team management, or higher up the food chain. If there is no culture of quality among your developers, this is a problem that management needs to address. And if they won't address it, then you need to step carefully with your co-workers.

Stephen C
  • 698,415
  • 94
  • 811
  • 1,216
  • Sonar, now that I think about it, is under discussion for future integration. Our developers are not necessarily wilful in their coding. It's the lack of a code assessment/auditing process in the past that resulted in the code quality reduced. We are not on "red alert", but simply issues are discovered randomly after long time they are committed, and new continue to appear. The fascist approach (which deserved mentioning) is not feasible as, like other people noted, you might be in a hurry and preventing from checking in low-quality but working code isn't applicable. – usr-local-ΕΨΗΕΛΩΝ Dec 21 '14 at 13:33
0

You could use checkstyle's regexp checks to fail anything containing System.out, or printStackTrace. There are some unlikely corner cases where it could produce false positives/negatives (e.g. if there were other types with a printStackTrace method).

Nicholas Daley-Okoye
  • 2,267
  • 1
  • 18
  • 12
0

You could write a custom detector in FindBug and integrate it with your build. I found a link to write custom detector in FindBug

MohamedSanaulla
  • 6,112
  • 5
  • 28
  • 45