Code reviews? - Page 2

Do you have a question? Post it now! No Registration Necessary

Translate This Thread From English to

Threaded View
Re: Code reviews?
Quoted text here. Click to load it

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%?

Quoted text here. Click to load it

Idiots usually have delusions.

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

Quoted text here. Click to load it

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.

Quoted text here. Click to load it

That is I hope not in debate by anyone.

Quoted text here. Click to load it

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

Quoted text here. Click to load it

Yes.

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

Re: Code reviews?


Quoted text here. Click to load it

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.  :)


Re: Code reviews?
says...
Quoted text here. Click to load it
Everyone who programs in C NEEDS lint

Robert


Re: Code reviews?

Quoted text here. Click to load it
You use lint to save you time tracking down your careless errors. It can
also pick out your bad habits. Once you have cleaned up your code with lint
you can get on with the real debugging.

Peter



Re: Code reviews?
snipped-for-privacy@newprovidence.demon.co.uk says...
Quoted text here. Click to load it

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

Re: Code reviews?
eolusdevelopment.cm> writes
Quoted text here. Click to load it

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

They do different jobs.

Quoted text here. Click to load it

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

Quoted text here. Click to load it

Likewise.


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

Re: Code reviews?
says...
Quoted text here. Click to load it
I rather glossed over that point didn't I ?  

I really was referring to syntax checking in this case though.  That
portion of analysis that each does (if for different purposes) to confirm
the code is valid and catches the simple typos (extra closing brackets,
dropped ; etc..).  Some compilers do a truly lousy job of identifying the
original error.  Lint (PC Lint at least) is as good as the best compilers
I've worked with at that identification.

Lint has many more important strengths (especially given good compilers)
but it's a very practical advantage with compilers that have had little
thought given to their error detection.

Robert

Re: Code reviews?
eolusdevelopment.cm> writes
Quoted text here. Click to load it

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       www.phaedsys.org \/\/
\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/

Re: Code reviews?


Quoted text here. Click to load it

Amen to that.

Also, "I can't use someone else's library. It doesn't use my stylistic
conventions for naming things, I don't know where it's been, and it'll
take longer to find the bugs in it than to write my own from scratch."

cheers, Rich.

--
rich walker         |  Shadow Robot Company | snipped-for-privacy@shadow.org.uk
technical director     251 Liverpool Road   |
We've slightly trimmed the long signature. Click to see the full one.
Re: Code reviews?

Quoted text here. Click to load it

I thought the truth was that sometimes you feel it may not be worthwhile
doing a review. In actuallity it is always worthwhile and you may get to
the point of wishing you had when an un-reviewed bit of code bites you back
hard. Of course, up to now you may have just got away with it but one
day...... ;>

Quoted text here. Click to load it

If you keep to small simple modules of documentation and code, you can
merely drip feed it through the reviewer. Other respondents have already
mentioned the benefits of keeping it simple.
 
Quoted text here. Click to load it

OK if it is a product that has no safety implications.
 
Quoted text here. Click to load it

Is that why we get glitches on the phone line from time to time??? ;>
 
Quoted text here. Click to load it

No screams. Just the gentle sounds of "tut, tut, tut".

--
********************************************************************
We've slightly trimmed the long signature. Click to see the full one.
Re: Code reviews?
Paul E. Bennett said
Quoted text here. Click to load it

Really?  I certainly didn't say or imply that.


Quoted text here. Click to load it

You can't if there is "the reviewer" doesn't exist.   Keeping things
simple is great, but there are real-world cases where you just don't
have the time, money, or luxury for a review.

Quoted text here. Click to load it

Products with safey implications are of course in a different category.


Quoted text here. Click to load it

Client asks you to do the software for a new product - the budget is
tight, the schedule almost impossible.  You're presented with a choice
- take the job and do your best or refuse the job.  "Tut, tut," doesn't
enter into the equation.  The fact that you have a teenaged son that
eats a lot might weigh heavily on your mind.

Btw, I just finished one of those jobs.


Casey

Re: Code reviews?
Quoted text here. Click to load it

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       www.phaedsys.org \/\/
\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/

Re: Code reviews?
Quoted text here. Click to load it

What about products that cause a lot of damage. If a CNC machine
drills holes in the wrong spot, it can ruin quite a bit, truly
upsetting people.

This is all kindergarten talk. We all know that some parts of
our software are more critical than others. And not all bugs
have desastrous effects. Nobody ships code without at least
some confidence, and most 'bugs' are only a nuisance, not more.
Hell, I even ship code with known bugs. Work in progress.

A few months ago, somewhere in the UK, an ATM machine was
spitting out twice the amount of money. In no-time the
entire village was queing up in front of the machine.
Software bug? Nah. A bank employee had swapped the bins
by mistake, and 20 pound notes were ejected instead of 10
pounds.

I am a bit amused that the majority here exclusively works
on applications that launch rockets, keep satelites in
orbit, control heart-lung machines, or are used in airplanes
(and I am not referring to the sensor that detects if the
toilet seat is up or down) and god knows what have you.
As a result, malfunctioning software always let the rockets
explode, satelites get lost, patients die by the dozen,
and airplanes burry themselves in the ground. For some
reason we never hear of that button that pops up the
wrong menu and confuses the operator, the vending machine
that thought it was empty while not, the weird message in
the logfile, the panel light that didn't go off, the
christmas lights that sometimes changes it's blinking
pattern, etc....

--
Thanks, Frank.
(remove 'x' and 'invalid' when replying by email)



--
Thanks, Frank.
(remove 'x' and 'invalid' when replying by email)



Re: Code reviews?

Quoted text here. Click to load it

Safety is an issue in all CNC machines so I would hope is not the type of
product that does not see a code review, not only for the basic executive
code but also for the machining code to produce the product. In the latter
I know there is some tool assistance. Not only is it expensive to damage
the tools but it can also launch shrapnel if it shatters (hende you are
then hoping that the guards are actually closed and adequate to the task of
catching the bits).
 
Quoted text here. Click to load it

True.


I would hope that it is more than merely some confidence. I tend to require
a greater element of certainty about what I ship. Just for my own peace of
mind if nothing else.

Quoted text here. Click to load it

If I know I have a bug in the code it is not shipped until the bug has been
removed. Mind you, most of them are eliminated very early in the coding
segment of development.
 
Quoted text here. Click to load it


Some examples of my involvements:-
   Hydrolastic Mixing Facility (25 years at issue 1 - 6800 Assembly)
   Gas Analysis (for automotive industry - GA Assemmbly)
   Peripheral Test Programs (Pr1me Assembly, BASIC and Fortran IV)
   Gas & Oil Production Well Head Controls (WesDAC SCADA)
   Railway Signalling Equipment (Ada)
   Automated Battery Charging Systems (6309 Forth)
   Paraplegic Electric Wheelchair (RTX2001 Forth)
   Nuclear Fuel Re-processing equipment
   Train Safety Monitoring System
   Coal Stacker Reclaimer (Mitsubishi PLC)
   Warship Compass Data Distribution System (D3 and 1553B)
   Army Tank Equipment Test Rigs (Gun and Suspension) (Forth)
   Nuclear Power Plant Fuel Handling and Recycling Equipment (S80)
   Plutonium Pick and Place Crane (6309 Assembly, Literal and Forth)
   Medical Anesthesia Ventilator Software Certification (Forth)
   Multihead Weighing System (Forth)
   Medical Sterilisation Fluid Manufacturing Facility (Forth)
   Fusion Reactor Fuelling Systems (Allen Bradley PLC and Forth)

Naturally I wasn't on my own for all of those. Some of the projects are
only done from within a larger company. However, some of the more critical
ones were handled by a team of between 5 and 9 people filling all the
engineering and project management roles.

Quoted text here. Click to load it

No matter how good we all become at our jobs, no matter how perfectly
produced and debugged our software becomes, there is always the chaotic
element that will unravel our best efforts. However, the chaotic element
does not need to be given a helping hand to disrupting our lives just
because we failed to review our work and let the bug we should have spotted
through the net.

--
********************************************************************
We've slightly trimmed the long signature. Click to see the full one.
Re: Code reviews?

Quoted text here. Click to load it
<snip>
Quoted text here. Click to load it
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

Re: Code reviews?

Quoted text here. Click to load it

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.

--
********************************************************************
We've slightly trimmed the long signature. Click to see the full one.
Re: Code reviews?


Quoted text here. Click to load it

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.

 


Re: Code reviews?
Quoted text here. Click to load it

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       www.phaedsys.org \/\/
\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/

Re: Code reviews?
Quoted text here. Click to load it
Can't say I know the book :>)

Still, I can't imagine a technique to review 20.000 - 25.000 line of
(commented) C code in 4 hours. We found out the hard way that the review
process wasn't good enough. During integration testing we found lots of
errors that should have been detected in the reviews.

But my main point was, that reviewing has to be taken into account
during the planning stage, and not just a token to please management.

Regards,
    Hans Bus

Re: Code reviews?

Quoted text here. Click to load it

That is just too much of a big chunk all at once. Wasn't this code in a
series of smaller sub-routines? The main bulk of the review work is
actually conducted outside of the meeting room. Pre-circulate the material
to be reviewed (the moderators) and the meeting collects the list of
problems areas to be revisited by the developer.
 
Quoted text here. Click to load it

Agreed. It should be part of the scheduled time and significantly so. No
review should meeting should be longer than 2 hours. Many of mine take less
than an hour but I do have a lot of them.


--
********************************************************************
We've slightly trimmed the long signature. Click to see the full one.

Site Timeline