<div class="gmail_quote">On Wed, Dec 30, 2009 at 11:18 AM, Tom Melendez <span dir="ltr">&lt;<a href="mailto:tom@supertom.com">tom@supertom.com</a>&gt;</span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
<br>
2. You find something wrong, then what?  And what if it is not<br>
&quot;really&quot; wrong (Jones uses a selection sort but sould have used an<br>
insertion sort, and no one told him beforehand).  Do we hold up the<br>
project until the engs figure out what is best, even though the code<br>
&quot;works&quot;?  Good luck explaining that to mgmt.  And politically, it can<br>
make Jones look bad unnecessarily, yet, as the new guy on the team, he<br>
might not be familiar with the standards, practices, etc. of your<br>
group.<br><br></blockquote><div>Most of the time you probably shouldn&#39;t make the change. However, a little //TODO (or @todo if you use a javadoc workalike)  means when that routine needs to be optimized you will find it faster. Also, Jones now knows to use an insertion sort next time. The code review can be an educational opportunity for next time.</div>
<div><br></div><div>If the code review makes Jones look bad then you either need to remove nontechnical management from the code review his peers need to have reasonable expectations.</div><div><br></div></div>