Last active
September 14, 2015 20:34
-
-
Save pestophagous/98adcb63c290ad782d8e to your computer and use it in GitHub Desktop.
coding guidelines and code-review checklist. view as html: https://rawgit.com/pestophagous/98adcb63c290ad782d8e/raw/ed388119fa6152271be1e6f45c9b522cb548a449/list.html
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| <html ><head><meta http-equiv="Content-Type" content="text/html;charset=utf-8"><meta name="viewport" content="initial-scale=0.75"> | |
| <style type="text/css"> | |
| .bugviewWidth { width: 757px; } | |
| div.ixBug { width: 187px; } | |
| #bugviewContainerSide { width: 170px; } | |
| .bugFields,.bugevents { margin-left: 192px; } | |
| html > body { | |
| /* NON-IE ONLY: Keep vertical scrollbar around, regardless of page height */ | |
| overflow-x: auto; | |
| overflow-y: scroll; | |
| } | |
| </style> | |
| <!-- Conditional Comment for IE-only styles...will override the imported styles.asp definitions. --> | |
| <!--[if IE]> | |
| <style> | |
| table.bugs nobr | |
| { | |
| padding-right: 0; | |
| } | |
| body .popupRow | |
| { | |
| display: block; | |
| } | |
| </style> | |
| <![endif]--> | |
| <!--[if lt IE 7]> | |
| <style> | |
| #Menu_Favorites:hover, #Menu_Favorites:active, #Menu_Favorites:focus { | |
| position: relative; | |
| left: -1px; | |
| } | |
| </style> | |
| <![endif]--> | |
| </head><!-- see http://naeblis.cx/rtomayko/weblog/PerSiteUserStyles --> | |
| <body onunload="bodyUnload();" id="www-fogcreek-com-fogbugz" class="webkit"><div style="position: absolute; text-align: left; border-width: 1px; border-style: solid; border-color: rgb(181, 178, 181) rgb(0, 0, 0) rgb(0, 0, 0) rgb(181, 178, 181); font-family: 'Lucida Grande'; font-size: 11px; max-height: 260px; overflow-y: auto; overflow-x: hidden; display: none; background-color: rgb(255, 255, 255);" id="idDropList_idScheduleItemInput_oDropList" tabindex="-1"></div><div style="position: absolute; text-align: left; border-width: 1px; border-style: solid; border-color: rgb(181, 178, 181) rgb(0, 0, 0) rgb(0, 0, 0) rgb(181, 178, 181); font-family: 'Lucida Grande'; font-size: 12px; max-height: 260px; overflow-y: auto; overflow-x: hidden; display: none; background-color: rgb(255, 255, 255);" id="idDropList_searchFor_oDropList" tabindex="-1"></div><div id="menuDisappearingMaskDiv" style="display:none;position:absolute;top:0px;left:0px;height:0px;width:0px;z-index:1;" onmousedown="javascript:return (theMgr.hideAllPopups() || KeyManager.cleanupOnClick());"> </div><div id="idClickmaskSnippetDiv" style="display:none;position:absolute;top:0px;left:0px;height:0px;width:0px;z-index:1;" onmousedown="hideSnippetHelper(); hideSnippetWarning(); return false;"> </div><span id="kbCursor" style="display:none;position:absolute;z-index:1"><img src="images/cursor.gif" alt=">"></span> | |
| <table class="Basic" style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black; width: 1021px"> | |
| <tbody> | |
| <tr> | |
| <th style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black; background-color: rgb(179, 238, 238)"> | |
| <span style="font-size:20px; line-height:20px">Before You Commit:</span></th> | |
| <th style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black; background-color: rgb(179, 238, 238); width: 142px"> | |
| </th> | |
| </tr> | |
| <tr> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black"> | |
| <p> | |
| <strong>Assume that all code will be reviewed.</strong></p> | |
| <p> | |
| (Sooner or later it all will. It all needs maintenance, so someone eventually re-reads all of it.) </p> | |
| <p> | |
| <strong>Treating all code as though someone will immediately read it and try to follow it leads to better code.</strong></p> | |
| </td> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black; width: 142px"> | |
| </td> | |
| </tr> | |
| <tr> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black"> | |
| <p> | |
| Write a commit message that makes sense.</p> | |
| <p> | |
| Think of what the code review assignment email looks like! Your note is shown in isolation. Make it clear. </p> | |
| <p> | |
| (In other words, not just "<em>continuation of yesterday's work</em>" -- because "yesterday's work" will likely not be shown adjacent to this comment in the assignment email!)</p> | |
| </td> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black; width: 142px"> | |
| </td> | |
| </tr> | |
| <tr> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black"> | |
| <p> | |
| Commit often! If some functionality is not yet complete, just place a 'wxFAIL' or a 'wxFAIL_MSG' to warn anyone who hits that code.</p> | |
| <p> | |
| Committing often is important because it makes it easier for all of us to merge each other's changes before anyone diverges enough to make merging a significant hassle.</p> | |
| </td> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black; width: 142px"> | |
| </td> | |
| </tr> | |
| <tr> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black"> | |
| <p> | |
| The commit message should be tagged with a BugzID number.</p> | |
| <p> | |
| If there is no relevant BugzID that you can use, then consider opening a new bugtracker case. (Obvious exceptions would be small commits where a function was renamed for clarity or something like that. No sense opening a case for that.)</p> | |
| </td> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black; width: 142px"> | |
| </td> | |
| </tr> | |
| <tr> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black"> | |
| <p> | |
| <strong>Only one bug/feature per commit. No mixing unrelated work into one commit!</strong></p> | |
| <p> | |
| If you are struggling with how to do this, then you probably need to study more git manuals and git tutorials. Git provides an enormous amount of flexibility in selecting what to commit.</p> | |
| </td> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black; width: 142px"> | |
| </td> | |
| </tr> | |
| <tr> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black"> | |
| If your changes affect submodules (sub-repos), then commit them and synchronize everything correctly (e.g. Licensing-Repo and Common-Util-Repo)</td> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black; width: 142px"> | |
| </td> | |
| </tr> | |
| <tr> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black"> | |
| If you are adding a significant new feature, then *please* add new files! It is a good thing when we can extend the program by adding new classes instead of tinkering (and possibly breaking) existing ones.</td> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black; width: 142px"> | |
| </td> | |
| </tr> | |
| <tr> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black"> | |
| If the code is based on an internet or book resource, please note this in comments.</td> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black; width: 142px"> | |
| </td> | |
| </tr> | |
| <tr> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black"> | |
| In new files, add the proper comment-block at top. (If the file is generated by DialogBlocks, then nevermind. Leave it as is.)</td> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black; width: 142px"> | |
| </td> | |
| </tr> | |
| <tr> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black"> | |
| If a new function is reusable, write it accordingly and put it in Common-Util-Repo.</td> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black; width: 142px"> | |
| </td> | |
| </tr> | |
| <tr> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black"> | |
| <p> | |
| Source files in 'Common-Util-Repo' and 'Licensing-Repo' should not pound-include Main-Application files. General rule: library code cannot pound-include headers that are application headers from the EXE that links to the library.</p> | |
| </td> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black; width: 142px"> | |
| </td> | |
| </tr> | |
| <tr> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black"> | |
| If you use a wxWidgets or Qt class or function that is new to you, read the WHOLE documentation page for it. Playing with the wx or Qt sample is recommended, too.</td> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black; width: 142px"> | |
| </td> | |
| </tr> | |
| <tr> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black"> | |
| When editing a class or function, read all comments for the class and function, and <strong>be sure to update or remove any comments rendered inaccurate by your changes</strong>. Be sure to check header and cpp file for comments. (It's good to read all commentary anyway -- sometimes it leads you to better and/or fewer changes by calling your attention to something you missed in pure code-reading.)</td> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black; width: 142px"> | |
| </td> | |
| </tr> | |
| </tbody> | |
| </table> | |
| <p> | |
| </p> | |
| <p> | |
| </p> | |
| <table class="Basic" style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black; width: 1021px"> | |
| <tbody> | |
| <tr> | |
| <th style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black; background-color: rgb(179, 238, 238)"> | |
| <span style="font-size:20px; line-height:20px">Style Guidelines - Strive to Make All Code Follow These:</span></th> | |
| <th style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black; background-color: rgb(179, 238, 238); width: 142px"> | |
| </th> | |
| </tr> | |
| <tr> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black"> | |
| With any if/else blocks, try to put the SHORTEST logic *first* (in the 'if' portion) and the longer logic in the 'else'. (Otherwise, when you read down through a looooong if-block and then notice "oh hey there is an <strong>else</strong> here" then you sometimes forget <strong>else what</strong>!!)</td> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black; width: 142px"> | |
| </td> | |
| </tr> | |
| <tr> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black"> | |
| When using '#ifdef' or '#ifndef', always put a comment on the '<strong>#endif</strong>' (in other words: <em>on the closing end</em>) that shows which '#if' is being closed/terminated !!</td> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black; width: 142px"> | |
| </td> | |
| </tr> | |
| <tr> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black"> | |
| Never put "using namespace" in a header. Also: when putting this in a cpp file, put it <strong>after</strong> all pound-includes.</td> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black; width: 142px"> | |
| </td> | |
| </tr> | |
| <tr> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black"> | |
| Never store full-blown objects in STL containers. Instead, store shared_ptr(s) to the objects.</td> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black; width: 142px"> | |
| </td> | |
| </tr> | |
| <tr> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black"> | |
| Do your best to avoid creating any new "<a href="http://sourcemaking.com/refactoring/data-class" style="text-decoration: none; color: rgb(15, 84, 145)" rel="nofollow">Data Class</a>". A Data Class is *not* object-oriented and does not perform proper encapsulation/data-hiding. We have some already, but avoid them in new code.</td> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black; width: 142px"> | |
| </td> | |
| </tr> | |
| <tr> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black"> | |
| Do not indulge in "<a href="http://sourcemaking.com/refactoring/speculative-generality" style="text-decoration: none; color: rgb(15, 84, 145)" rel="nofollow">Speculative Generality</a>"</td> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black; width: 142px"> | |
| </td> | |
| </tr> | |
| <tr> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black"> | |
| Any time you encounter "<a href="http://stackoverflow.com/questions/6609296/code-smells-message-chains-vs-middle-man" style="text-decoration: none; color: rgb(15, 84, 145)" rel="nofollow">Message Chains</a>", try to see if the chain can be shortened!</td> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black; width: 142px"> | |
| </td> | |
| </tr> | |
| <tr> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black"> | |
| <strong>Never put a 'return' in the middle of a function! Strive for only a single 'return' per function.</strong> One small exception is an "early return" near the very top, simply to avoid the nesting of an extra "if ( some_fatal_condition )" pushing nesting into the whole function body.</td> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black; width: 142px"> | |
| </td> | |
| </tr> | |
| <tr> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black"> | |
| Remember that the string "Trim" function in wxWidgets is silly. We usually want to call all this: some_var.Trim(false).Trim(true)</td> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black; width: 142px"> | |
| </td> | |
| </tr> | |
| <tr> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black"> | |
| Instead of adding a bunch of wxLogDebug calls that are all related, *please* create a new trace mask instead.</td> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black; width: 142px"> | |
| </td> | |
| </tr> | |
| <tr> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black"> | |
| Do not commit commented-out code. If the code is no longer needed, remove it. It is safely stored in git, anyway. If you are strongly tempted to leave it in, then consider <strong>replacing</strong> it with a comment such as: "<em>some apparently obsolete code used to be here. please refer to a version of this file as of 15-March-2010 or earlier to retrieve the old code from version control</em>."</td> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black; width: 142px"> | |
| </td> | |
| </tr> | |
| <tr> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black"> | |
| No function body should be all-on-one-line, like '{ return true; }' -- this sometimes makes it impossible to set a breakpoint in there.</td> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black; width: 142px"> | |
| </td> | |
| </tr> | |
| <tr> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black"> | |
| For comments about how to *use* your function or class, please use doxygen-style comments.</td> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black; width: 142px"> | |
| </td> | |
| </tr> | |
| <tr> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black"> | |
| Avoid deep (tall) inheritance hierarchies. </td> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black; width: 142px"> | |
| </td> | |
| </tr> | |
| <tr> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black"> | |
| Do not fear "wide" inheritance. A wide inheritance hierarchy, especially a single depth of many sibling children, is acceptable and many times is a sign of <strong>good</strong> polymorphism.</td> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black; width: 142px"> | |
| </td> | |
| </tr> | |
| <tr> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black"> | |
| Is "virtual" tacked onto a function for a reason? We don't need to blindly add virtual everywhere. (Dtors are the exception to the rule. It's ok to religiously put 'virtual' on all dtors. It would be dangerous if we later subclassed and forgot to add it then.)</td> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black; width: 142px"> | |
| </td> | |
| </tr> | |
| <tr> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black"> | |
| Avoid usage of "friend class" in new code. In extreme cases you may friend a single function. (But passing an anonymous 'wrapped' functor using boost::function is perhaps a better way to allow other code to 'call' your private function.)</td> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black; width: 142px"> | |
| </td> | |
| </tr> | |
| <tr> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black"> | |
| Do not use multiple inheritance EXCEPT when using pure virtual pure abstract "interfaces." If we are only dealing with pure abstract interfaces, we can be a bit bolder.</td> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black; width: 142px"> | |
| </td> | |
| </tr> | |
| </tbody> | |
| </table> | |
| <p> | |
| </p> | |
| <div> | |
| </div> | |
| <table class="Basic" style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black; width: 1021px"> | |
| <tbody> | |
| <tr> | |
| <th style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black; background-color: rgb(179, 238, 238)"> | |
| <span style="font-size:20px; line-height:20px">Checklist. Answer These For Each Code Review:</span></th> | |
| <th style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black; background-color: rgb(179, 238, 238); width: 142px"> | |
| </th> | |
| </tr> | |
| <tr> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black"> | |
| <strong>Is there sanity-checking on return values?</strong> If something even *potentially* returns an error value, *always* do sanity checking on it.</td> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black; width: 142px"> | |
| </td> | |
| </tr> | |
| <tr> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black"> | |
| <strong>Are all new data-members private?</strong></td> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black; width: 142px"> | |
| </td> | |
| </tr> | |
| <tr> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black"> | |
| <strong>Even new data-members made by DialogBlocks should usually be private.</strong> DialogBlocks does not insert the 'private:' line, so you must do it yourself.</td> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black; width: 142px"> | |
| </td> | |
| </tr> | |
| <tr> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black"> | |
| <strong>Do all the new pound-include directives appear in CPP files only?</strong> Use forward declarations and shared_ptr in headers to help with this.</td> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black; width: 142px"> | |
| </td> | |
| </tr> | |
| <tr> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black"> | |
| <strong>Are headers for new classes "self-sufficient"?</strong> What this means in practice is that in any newly-added CPP file, the very topmost pound-include *must* be the #include for the header that goes with that cpp. This compiler will then scan that header <em>first</em>, before having pulled in any other 'wx' or 'boost' things, which then proves that your new header is "self-sufficient" (i.e. not secretly depending on some other item just 'coincidentally' getting pound-included beforehand). [See "Coding Standards" book, item 23.]</td> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black; width: 142px"> | |
| </td> | |
| </tr> | |
| <tr> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black"> | |
| <strong>There is no duplicate code?</strong> Even four lines of code are enough to make a helper function.</td> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black; width: 142px"> | |
| </td> | |
| </tr> | |
| <tr> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black"> | |
| <strong>Any new compiler warnings?</strong> Code must not introduce any new compiler warnings</td> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black; width: 142px"> | |
| </td> | |
| </tr> | |
| <tr> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black"> | |
| <strong>Code must not cause tests to fail.</strong> Either fix the code or update the test. Otherwise, inform everyone why a test must temporarily fail.</td> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black; width: 142px"> | |
| </td> | |
| </tr> | |
| <tr> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black"> | |
| <strong>If a copy ctor changed, then did the operator= function change accordingly?</strong></td> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black; width: 142px"> | |
| </td> | |
| </tr> | |
| <tr> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black"> | |
| <strong>If the operator= changed, then did the copy ctor change accordingly?</strong></td> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black; width: 142px"> | |
| </td> | |
| </tr> | |
| <tr> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black"> | |
| <strong>If a member variable is added or moved, then did the member initialization list in the ctor get updated?</strong></td> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black; width: 142px"> | |
| </td> | |
| </tr> | |
| <tr> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black"> | |
| <strong>If the keyword 'new' is used anywhere other than in a shared_ptr, then are we certain no leaks got introduced?</strong></td> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black; width: 142px"> | |
| </td> | |
| </tr> | |
| <tr> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black"> | |
| <strong>Was all usage of printf-style string formatters done correctly?</strong> All '%' placeholders must be correctly matched to the corresponding variable parameter. Achieving crashes and (worse!) corruption is all too easy with formatters.</td> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black; width: 142px"> | |
| </td> | |
| </tr> | |
| <tr> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black"> | |
| <strong>If the type of a variable was changed, then did printf(s) change accordingly?</strong> If a variable changed (from int to bool, or from int to float, for example), then watch out to see if this variable is passed to any printf-style function that relied on the prior type!</td> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black; width: 142px"> | |
| </td> | |
| </tr> | |
| <tr> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black"> | |
| <strong>Are sufficient assertions utilized to flag assumptions and potential misuse/misbehavior of code? </strong> (but also see next item)</td> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black; width: 142px"> | |
| </td> | |
| </tr> | |
| <tr> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black"> | |
| <strong>Do we need to choose a RELEASE-MODE error instead of an assertion anywhere?</strong> (assertions disappear in release build)</td> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black; width: 142px"> | |
| </td> | |
| </tr> | |
| <tr> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black"> | |
| <strong>If a ctor takes exactly one parameter, then did we add 'explicit'?</strong></td> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black; width: 142px"> | |
| </td> | |
| </tr> | |
| <tr> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black"> | |
| <strong>Did we explicitly disallow copying (unless there is an exceptional need)?</strong> (this is to be done using DECLARE_NO_COPY_CLASS or boost::noncopyable)</td> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black; width: 142px"> | |
| </td> | |
| </tr> | |
| <tr> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black"> | |
| <strong>Did we use const wherever possible?</strong> (member functions, parameters, some local variables, etc)</td> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black; width: 142px"> | |
| </td> | |
| </tr> | |
| <tr> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black"> | |
| <strong>Did we avoid preprocessor usage?</strong> (aka avoid "macros")</td> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black; width: 142px"> | |
| </td> | |
| </tr> | |
| <tr> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black"> | |
| <strong>Did we avoid default values in member function parameter declarations?</strong> (<em>especially in virtual inherited functions</em>)</td> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black; width: 142px"> | |
| </td> | |
| </tr> | |
| <tr> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black"> | |
| <strong>Is casting used correctly? (<em>even better if we can code without any need for casting...</em>) </strong>Be very wary of casting! Refer to Effective c++ third edition, Item 27</td> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black; width: 142px"> | |
| </td> | |
| </tr> | |
| <tr> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black"> | |
| <strong>Did we avoid the 'inline' keyword? It is only a compiler hint and is rarely needed or effective.</strong> Refer to Effective c++ third edition, Item 30</td> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black; width: 142px"> | |
| </td> | |
| </tr> | |
| <tr> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black"> | |
| <strong>Are (non-template) function bodies kept strictly to the cpp file?</strong> (since we just said the 'inline' is not to be used), there is no reason to put function <strong>bodies</strong> in the header. Please keep them in the cpp. However, see next item for the template case.</td> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black; width: 142px"> | |
| </td> | |
| </tr> | |
| <tr> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black"> | |
| <strong>Are template definitions (bodies) kept *in* the header, together with declarations?</strong> All new template classes should have a single header file per class and no cpp file. This follows the "inclusion model" in <a href="http://amzn.com/0201734842" rel="nofollow">6.1.2 of this book (C++ Templates)</a>.</td> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black; width: 142px"> | |
| </td> | |
| </tr> | |
| <tr> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black"> | |
| <strong>Have we verified there are no bare literal magic numbers??!!</strong> If some interesting number is required, then at least make some const value with an explanatory name, and probably a comment, too.</td> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black; width: 142px"> | |
| </td> | |
| </tr> | |
| <tr> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black"> | |
| <strong>Are we using "<a href="http://sourcemaking.com/refactoring/introduce-explaining-variable" style="text-decoration: none; color: rgb(15, 84, 145)" rel="nofollow">Introduce Explaining Variable</a>" whenever possible?</strong></td> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black; width: 142px"> | |
| </td> | |
| </tr> | |
| <tr> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black"> | |
| <strong>Are all variables always initialized?</strong> Do not ever leave something like: "int factor;" (instead, type an "=" after the var name and initialize!)</td> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black; width: 142px"> | |
| </td> | |
| </tr> | |
| <tr> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black"> | |
| <strong>Have we only returned 'safe' objects and data from any new functions? Do not return pointers to private member variables! </strong> (some existing code does this. avoid it in new code.)</td> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black; width: 142px"> | |
| </td> | |
| </tr> | |
| <tr> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black"> | |
| <strong>Did we do boundary checking before any and all '[i]' access?</strong> (arrays/vectors/deques/etc)? Good loop conditions can qualify as sufficient checking.</td> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black; width: 142px"> | |
| </td> | |
| </tr> | |
| <tr> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black"> | |
| <strong>Are variable names clear?</strong> (Don't be afraid to suggest a rename to the original coder.)</td> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black; width: 142px"> | |
| </td> | |
| </tr> | |
| <tr> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black"> | |
| <strong>Are function names clear?</strong> (Don't be afraid to suggest a rename to the original coder.)</td> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black; width: 142px"> | |
| </td> | |
| </tr> | |
| <tr> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black"> | |
| <strong>Are comments clear?</strong> (Don't be afraid to suggest a rewording to the original coder.)</td> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black; width: 142px"> | |
| </td> | |
| </tr> | |
| <tr> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black"> | |
| <strong>Are error messages and assertion-failure messages clear?</strong> (Don't be afraid to suggest a rewording to the original coder.)</td> | |
| <td style="border-collapse: collapse; border-top-width: 1px; border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: black; border-right-color: black; border-bottom-color: black; border-left-color: black; width: 142px"> | |
| </td> | |
| </tr> | |
| </tbody> | |
| </table> | |
| </body></html> |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment