0

There're several new c++ guys working in our team, so too much ugly code everyday! I hate those functions using readonly string, STL containers as parameters in, but without const reference!!! I'm crazy!!!

Is there any static code checker that can find these ugly code? I need such a tool used in our makefile.

BenMorel
  • 34,448
  • 50
  • 182
  • 322
Acewind
  • 133
  • 9
  • 6
    You're probably better off with peer review. – AndiDog Aug 18 '12 at 08:57
  • You should probably write down some basic guidelines for them to follow. I don't think that a static code checker would identify those as errors. It sounds like you hired people who have never actually used C++ outside of college courses. – Cubic Aug 18 '12 at 09:14
  • 2
    Static analysis will not teach them to be better developers. You need to invest in real training either from the team or dedicated classes. Code reviews imperative. – tenfour Aug 18 '12 at 09:33

2 Answers2

3

Yeah, it's unlikely that "bad code" can be prevented with automated tools.

For myself, and I'm also doing this at my workplace, I've always turned on as many warnings as possible (usually by enabling a high level of warnings and only turning off the 'obviously dumb' warnings; g++ being the only exception since it doesn't have an option to turn on everything, so I do -Wall, -Wextra and a whole bunch of other -W, and occasionally go through the manual to see whether new warnings have been added).

I also compile with -Werror or /WX. Unfortunately, while Linux and Windows headers seem to be rather clean by now, I get stupid warnings about things like bad casts or incorractly used macros from boost headers. 3rd party libraries are often badly written wrt to warnings.

As for static analysis tools, I did try cppcheck and clang (both of which are free, which is why I tried them). Wasn't thrilled about either of them; I'm still planning to add some support for one or both to my build software, but it has rather low priority. One of the two (don't remember which one) actually found SOMETHING: an unnecessary assignment, which any decent optimizer will remove anyway. I don't think that I'm such a perfect 0-bugs developer, so I'm blaming the tools. Still, I did remove that assignment :-)

If I'm not mistaken, the commercial VisualStudio versions have code analysis as well (At home I'm more of an Express guy, and I'm stuck with MacOS development at work); maybe that one is better. Or one of the other commercial tools; they have to offer SOMETHING for their money, after all.

There are still some additional free tools that I haven't tried yet; I have no idea how complete the http://en.wikipedia.org/wiki/List_of_tools_for_static_code_analysis#C.2FC.2B.2B list, but I hope to eventually try all the free tools that can handle C++.

For your problem in particular, Wi8kipedia describes "cpplint" as "cpplint implements what Google considers to be "best practices" in C++ coding". I have no idea what that means, but the Wikipedia page has a link to a "Google C++ Style Guide" pdf. Or you could just try it and see what it complains about :-)

Also, I probably wouldn't want to add such tools to the Makefile (unless you meant to imply that people still have to invoke "make check" to actually run it). Adding it to the source code repository to check new commits before allowing them is probably too time consuming (code analysis is pretty much "compiling with many extras", so it takes a good deal of time), but you could automatically run it every now and then.

Christian Stieber
  • 9,954
  • 24
  • 23
  • 2
    The problem with the Google style guide is that it documents what they *have to do* to maintain their very old code base. It is not a good guide to follow for new code. – Bo Persson Aug 18 '12 at 09:34
  • we have such a piece of code: void set_username(string user_name) { _user_name = user_name; }; which generates an exception in std::basic_string, std::allocator >::assign(std::basic_string, std::allocator > const&) () from /usr/lib64/libstdc++.so.6 Before any debug work to do, I am angry about the non-const reference of parameter "user_name". – Acewind Aug 18 '12 at 09:52
0

parfait or lint, google "Static Analysis Tools"

You could probably pick up some of these by using the -Wall flag if you are using GCC to get these as warnings.

ddoor
  • 5,819
  • 9
  • 34
  • 41