Code reviews?

I have different experience in the (last 12 years) in the embedded Telecom business. We ALWAYS did reviews. Unfortunately, if the planning allows you 4 hours preparation and 2 hours review meetings for a 3 month programming job, why bother, you get a lot of comments on comments, nobody had time to really analyse your code properly.

Morale : Serious reviewing takes a lot of time, plan for that.

Regards, Hans Bus

It's not quite the same thing, but one way you can get another "opinion" on your code (if it's C) is to run it through lint. Gimpel's PC-Lint is very good, and for open source, there is Splint (

Another way to approximate a real code review that is to use a checklist (there are many on the web). For each checklist item, if you go through the code just scanning for that one thing, you can do a pretty fair even on your own code.

I've done both of these on code I've written as a one-man-job. It's also a useful habit to get into when you get into the situation that you're actually getting real code reviews with other people, because your code will be that much better going into the review.

Ed

Except that really serious reviewing doesn't take that long at all. For guidance on the process of conducting all kinds of technical reviews and walkthroughs see the "Handbook of Walkthroughs, Inspections, and Technical Reviews: Evaluating Programs, Projects, and Products." by Daniel P. Freedman and Gerald M. Weinberg. Third Edition is ISBN 0-932633-19-6.

Yes, (sp)lint is nice to catch some pitfalls. But often I found it quite a hassle to configure it for a particular compiler.

I will look into that, and test myself ;)

"Frank Bemelman" schreef in bericht news:419f71b6$0$65124$ snipped-for-privacy@news.xsall.nl...

And I feel I will waste a lot of time on silly red tape:

g. Structuredness:

1) - of course, if it compiles it has an end.

In article , snipped-for-privacy@newprovidence.demon.co.uk says...

Yep, lint serves multiple purposes. - It's a good syntax checker, for many small embedded systems I've worked with it's orders of magnitude better than the compiler. I'm in the habbit of running through lint before the compiler. Usually the only compiler complaints I see are either chip specific or the compiler is in error. - It catches many simple errors that are syntactically correct. Usually if lint is complaining about it it is an error and if it is not there is usually a clearer way of expressing the intent that doen't cause lint to complain so.. - It leads to clearer code. One of the benefits of lint is over time it beats certain bad habbits out of you. - Since lint can check a lot of the detailed line by line picky points it reduces the load on reviewers to catch such things as order dependances, mismatched types etc, leaving them free to concentrate on the logic. When I last ran reviews it was a precondition of the review that the code lint clean (any exceptions noted and justified). - With Gimpels lint (I don't know about lc-lint) the use of strong type checking will catch type mismatches. Unresolvable type mismatches (ie w/o extensive casting) usually indicate an underlying flaw in the model used to represent the values used in the program. It's also capable of catching cross module errors which compilers don't (at least none I've worked with) catch (although proper header files make a difference here).

Consider lint an idiot savant reviewer. It doesn't tire and it doesn't grasp the logic of the specification but what it does check for it checks throughly and quickly. I've found that if lint is complaining about a lot of things that's usually a sign there are other problems as well.

These comments don't apply to the original Unix lint. It's been outpaced.

Robert

Excellent book.

It has been my experience that *not* doing reviewing and other kinds of software testing/quality assurance takes far more time than doing those things takes.

Another technique that I have had a lot of success with is doing assert-based programming during development, with a way to turn off the asserts at the end of the project.

It's easiest to start out with "-weak" and just work on that first, but you're right that there is work involved in configuring it to work with a particular compiler. Unfortunately, there is still no such notion as "interrupt" in standard C.

It works for me. I have also found that if I find an error (of any kind) the best thing to do is to look for more just like it. In some cases, I have written quick Perl scripts to automatically go through my code and search for certain kinds of things (e.g. free() of space allocated by alloca()). If I were more organized, I'd have saved each of those and made a nice collection, but I'm sad to report that I didn't have the forethought to do it. Maybe you will. :-)

Ed

Not the same thing. The question here is whether each *function* corresponds to a block of code. If you have an embedded system which has an LCD display, is the function responsible for driving that LCD recognizeable as a block of code? Or is the code which talks to the LCD littered throughout the source code? That's what that first one means.

In assembly language, it's much easier to make this kind of error, and in fact, there usually is no good reason to do so, as evidenced by the fact that you've never done it!

In any case, you need to create your *own* checklist which retains only relevant items. For example, "conforms to coding guidelines" might be irrelevant if you don't have any. However, I'd argue that the better fix to that particular one would be to invent and follow consistent coding guidelines. You probably don't write randomly formatted code now, but you might not necessarily have written down the "rules" for how you write code. That's all the coding guidelines are, and they're only important because they eliminate the distraction of poorly or inconsistently written code.

My rule of thumb: if you have a good reason to break a rule, do so.

Ed

Just document why so you don't wonder later. :)

Robert

In article , Frank Bemelman writes

That's why it is worth buying PC-Lint. Inn most places time==money. So a "free" tool can cost more than a commercial tool by the time you have set it up.

/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\ \/\/\/\/\ Chris Hills Staffs England /\/\/\/\/\ /\/\/ snipped-for-privacy@phaedsys.org

In article , WaldemarIII writes

No you shouldn't. It is a static analyser. It is not a beautifier. If you don't understand the difference you shouldn't be programming.

BTW what do you use for static analysis? You do, of course know, that static analysis will pick up 80% of the bugs and dynamic the other 20%?

Idiots usually have delusions.

I note that many who design the language swear by lint.

I am not surprised. If the comment above from your colleague was symptomatic of the company the boss probably left to find somewhere more professional.

That is I hope not in debate by anyone.

That is in the design notes and comments....

Yes.

/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\ \/\/\/\/\ Chris Hills Staffs England /\/\/\/\/\ /\/\/ snipped-for-privacy@phaedsys.org

In article , R Adsett writes

Interestingly I have found those who really know what they are doing swear by lint (or something similar) and would not program without some form of static analysis.

It is the cowboys and hackers who think they are so clever who don't use it. They *always* have their own special method that is better.

/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\ \/\/\/\/\ Chris Hills Staffs England /\/\/\/\/\ /\/\/ snipped-for-privacy@phaedsys.org

In article , R Adsett writes

The compiler is a translator. Lint is a static analyser.

They do different jobs.

That was I believe the original intention. It was intended to be used in makefiles every time you compiled.

Likewise.

/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\ \/\/\/\/\ Chris Hills Staffs England /\/\/\/\/\ /\/\/ snipped-for-privacy@phaedsys.org

In article , Casey writes

Then this is the perfect time to use lint... it will rip out many bugs and potential bugs almost as you write the code. Cuts down on a hell of a lot of debugging.

I too have done comms (SDH) systems. I also have a son (or two) who eats more than his own body weight per day, demands pocket money and borrows the car.... :-) /\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\ \/\/\/\/\ Chris Hills Staffs England /\/\/\/\/\ /\/\/ snipped-for-privacy@phaedsys.org

In article , Guy Macon writes

VERY TRUE I have seen it in action. Teams that use coding guidelines, static analysis and reviews had fewer bugs. Lint, specifically, cut out a lot of the noise which meant the few remaining bugs were easier to find.

This meant the project came in early! Saved the company a lot of money and we got on with the next one.

/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\ \/\/\/\/\ Chris Hills Staffs England /\/\/\/\/\ /\/\/ snipped-for-privacy@phaedsys.org

You have found a way to identify 100% of all bugs? Unbelievable!

It has been my experience that Static analysis finds the first 80% of the bugs, then dynamic analysis finds the second 80% of the bugs, leaving an additional 80% of the bugs undiscovered. :)

Yes, that's true, there's another reason why I avoid assembler ;)

Yes, over the years I have developed my own guidelines or adopted ones from others. While I haven't written them down, it does help a lot. But my latest bug would not have been caught by merely code review, not very likely anyway. Only by testing, I'd think. And I had tested it, but obviously not thoroughly enough ;)

Sounds like a good rule. Let's stick to that, before we break more ;)