We do software from small embedded up to server and backend. The "lower" parts (embedded uC) do the most code reviews. It lessens farther up. But they have the highest device numbers and the most problems correcting an error afterwards.
You don't want to waste a lot of time looking over individual lines
>of code -- assume the programmer is competent (else why did you
>hire him/her?). Rather, check for things that tend to slip
>between the cracks, identify and then *challenge* assumptions that
>may have been made, etc.
But enough errors are hidden in these individual lines. There are very few if any programmers who don't do any errors at that level. Especially in cases, when code is seldom used or the error is by luck handled by some other part of the program. So simple tests won't find it.
I learned to do CodeReviews just after coding before much tests have been done. So there will probably be errors in the code.
It is important that code reviews (and design reviews, in general)
>not be seen as "evaluations", "criticisms", etc. Personalities
>can make or break this entire process. The best environments
>that I've worked in had lots of really talented people (with
>*different* sets of talents) who could appreciate *your* skillset
>(as you appreciated theirs) and feel confident *challenging*
>something you've done knowing that (in case they are "wrong")
>they won't "look stupid" for doing so, etc. A sense of humor
>goes a long way to making this process work effectively.
Yes. Some problems will come to the mind of the presenter just by explaining it the the others. Some by questions from "outsiders" which never came up before.
And there's another benefit from codereview, at least if programmers from the same domain are involved. More people know the code and may work with it if needed in the future.
Code running in the servers, typically, is a lot "easier" to correct (update). They tend to have better user interfaces, connectivity, peripherals, etc.
But, I see little reason to differentiate between them when it comes to testing, reviews, etc. A bug is a bug regardless of where it manifests. Customer cares little about where the bug resides -- he just knows the product isn't doing what he *wants* it to do...
Of course! But, it takes a *lot* more time to go through line-by-line rechecking all these little details. Look at loop conditions, etc. -- "off by one". You don't want to tie up *several* people doing the work that one was
*supposed* to have done, competently.
[I guess it depends on the caliber of the people you have working for/with you]
Test each *module*. You should know what each "routine" is expected to do. It's easy to look at a *documented* interface and hypothesize the sorts of things that might have been omitted or poorly implemented. E.g., what if this is invoked with a NULL pointer? A cursory look through the code will tell you *if* the programmer even tried to address that situation.
I like to test while writing. My code will often have comments in it describing boundary conditions that could otherwise have tripped it up.
If you document interfaces, someone else can put together test cases to stress your code as soon as you think it is "done". This impacts the way you design your system -- since you want to be able to automate as much of the testing as possible, you don't mix interactive stuff with things that don't need to be interactive.
There is nothing wrong with being "chagrined" by a peer. I designed a processor many years ago while my best friend wrote the code for it. When it was built and ready for test, **nothing** worked. We each had boundless confidence in ourselves *and* the other party! So, the fact that it didn't do *anything* had us stumped.
The problem was eventually traced to different assumptions about addressing -- the hardware could only address "words" (since everything *was* a word) so the address field in all instructions expected a "word address". My friend, being used to writing in more conventional environments, had assumed *byte* addresses (so, all of his addresses had a rightmost bit of '0' -- my hardware had already made that *assumption*).
We *both* were chagrined at our assumptions -- though neither of us could realistically fault the other :-/
I find it most (self)educational to explain things to non-technical people. They ask questions that others would *assume* you had already addressed. The very act of trying to relate, by analogy, something to them causes you to examine your design in a level of detail that you might otherwise have glossed over.
This is the easiest way to "sell" code reviews to Management (who often see this activity as "not PRODUCING anything")
ElectronDepot website is not affiliated with any of the manufacturers or service providers discussed here.
All logos and trade names are the property of their respective owners.