Topicos Recents
-
Element click intercepted
Geral7 -
Dica de ferramentas para testes visuais
Geral4 -
Erro ao executar testes automatizados com o Chrome [Capybara + Selenium + Docker]
Geral1 -
Problemas com o nightwatch
Geral1 -
Ajuda com Curso
Geral5 -
Como você mapearia este elemento?
Geral12 -
Questionário sobre testes automatizados em aplicativos móveis
Geral4 -
A arte de desenvolver testes - Cucumber + Capybara
Artigos e Tutoriais44 -
Como abrir todos os link de uma página
Geral2 -
QA Analyst/Project Lead (Florianópolis/Remote)
Vagas1 -
QA Engineer - (Pinheiro)
Vagas1 -
Teste de Stress
Geral5 -
Tester que não programa, leia isso por favor.
Artigos e Tutoriais6 -
Executar tags em features diferentes no Cucumber
Geral1 -
QA, trate sua automação como software
Artigos e Tutoriais5 -
[Survey] - Testes automatizados em aplicativos móveis
Geral6 -
Cucumber para javascript Duvida
Artigos e Tutoriais2 -
Vagas QA PL e Sr - São Paulo
Vagas1 -
Episode 010 - The Automate or Die Special - The Evil Tester Show
Feed de Blogs e Posts2 -
Ruby In Tests
Artigos e Tutoriais7
Code Health: To Comment or Not to Comment?
-
This is another post in our Code Health series. A version of this post originally appeared in Google bathrooms worldwide as a Google Testing on the Toilet episode. You can download a printer-friendly version to display in your office.
By Dori Reuveni and Kevin Bourrillion
While reading code, often there is nothing more helpful than a well-placed comment. However, comments are not always good. Sometimes the need for a comment can be a sign that the code should be refactored.Use a comment when it is infeasible to make your code self-explanatory. If you think you need a comment to explain what a piece of code does, first try one of the following:
-
Introduce an explaining variable.
<table class=“my-bordered-table”>
<tbody>
<tr>
<td style=“background-color: #f4cccc; vertical-align: top; width: 50%;”>
<pre style=“background-color: #f4cccc; border: 0px; color: black; margin-bottom: 0; margin-top: 0; padding-bottom: 0; padding-top: 0; padding-left: 0;”>// Subtract discount from price.
finalPrice = (numItems * itemPrice)
- min(5, numItems) * itemPrice * 0.1;</pre></td>
<td style=“background-color: #d9ead3; vertical-align: top; width: 50%;”>
<pre style=“background-color: #d9ead3; border: 0px; color: black; margin-bottom: 0; margin-top: 0; padding-bottom: 0; padding-top: 0; padding-left: 0;”>price = numItems * itemPrice;
discount =
min(5, numItems) * itemPrice * 0.1;
finalPrice = price - discount;</pre></td>
</tr>
</tbody>
</table>
-
Extract a method.
<table class=“my-bordered-table” style=“width: 100%;”>
<tbody>
<tr>
<td style=“background-color: #f4cccc; vertical-align: top; width: 50%;”>
<pre style=“background-color: #f4cccc; border: 0px; color: black; margin-bottom: 0; margin-top: 0; padding-bottom: 0; padding-top: 0; padding-left: 0;”>// Filter offensive words.
for (String word : words) { … }</pre></td>
<td style=“background-color: #d9ead3; vertical-align: top; width: 50%;”>
<pre style=“background-color: #d9ead3; border: 0px; color: black; margin-bottom: 0; margin-top: 0; padding-bottom: 0; padding-top: 0; padding-left: 0;”>filterOffensiveWords(words);</pre>
</td>
</tr>
</tbody>
</table>
-
Use a more descriptive identifier name.
<table class=“my-bordered-table” style=“width: 100%;”>
<tbody>
<tr>
<td style=“background-color: #f4cccc; vertical-align: top; width: 50%;”>
<pre style=“background-color: #f4cccc; border: 0px; color: black; margin-bottom: 0; margin-top: 0; padding-bottom: 0; padding-top: 0; padding-left: 0;”>int width = …; // Width in pixels.
</pre></td>
<td style=“background-color: #d9ead3; vertical-align: top; width: 50%;”>
<pre style=“background-color: #d9ead3; border: 0px; color: black; margin-bottom: 0; margin-top: 0; padding-bottom: 0; padding-top: 0; padding-left: 0;”>int widthInPixels = …;</pre>
</td>
</tr>
</tbody>
</table>
-
Add a check in case your code has assumptions.
<table class=“my-bordered-table” style=“width: 100%;”>
<tbody>
<tr>
<td style=“background-color: #f4cccc; vertical-align: top; width: 50%;”>
<pre style=“background-color: #f4cccc; border: 0px; color: black; margin-bottom: 0; margin-top: 0; padding-bottom: 0; padding-top: 0; padding-left: 0;”>// Safe since height is always > 0.
return width / height;</pre></td>
<td style=“background-color: #d9ead3; vertical-align: top; width: 50%;”>
<pre style=“background-color: #d9ead3; border: 0px; color: black; margin-bottom: 0; margin-top: 0; padding-bottom: 0; padding-top: 0; padding-left: 0;”>checkArgument(height > 0);
return width / height;
</pre></td>
</tr>
</tbody>
</table>
There are cases where a comment can be helpful:
-
**Reveal your intent: explain why the code does something (as opposed to what it does).
<table class=“my-bordered-table” style=“width: 100%;”>
<tbody>
<tr>
<td style=“background-color: #d9ead3; vertical-align: top; width: 100%;”>
<pre style=“background-color: #d9ead3; border: 0px; color: black; margin-bottom: 0; margin-top: 0; padding-bottom: 0; padding-top: 0; padding-left: 0;”>// Compute once because it’s expensive.</pre>
</td>
</tr>
</tbody>
</table>**
-
**Protect a well-meaning future editor from mistakenly “fixing” your code. **<table class=“my-bordered-table” style=“width: 100%;”>
<tbody>
<tr>
<td style=“background-color: #d9ead3; vertical-align: top; width: 100%;”>
<pre style=“background-color: #d9ead3; border: 0px; color: black; margin-bottom: 0; margin-top: 0; padding-bottom: 0; padding-top: 0; padding-left: 0;”>// Create a new Foo instance because Foo is not thread-safe.</pre>
</td>
</tr>
</tbody>
</table>****
-
**Clarification: a question that came up during code review or that readers of the code might have. ****<table class=“my-bordered-table” style=“width: 100%;”>
<tbody>
<tr>
<td style=“background-color: #d9ead3; vertical-align: top; width: 100%;”>
<pre style=“background-color: #d9ead3; border: 0px; color: black; margin-bottom: 0; margin-top: 0; padding-bottom: 0; padding-top: 0; padding-left: 0;”>// Note that order matters because…</pre>
</td>
</tr>
</tbody>
</table>******
-
Explain your rationale for what looks like a bad software engineering practice. ******<table class=“my-bordered-table” style=“width: 100%;”>
<tbody>
<tr>
<td style=“background-color: #d9ead3; vertical-align: top; width: 100%;”>
<pre style=“background-color: #d9ead3; border: 0px; color: black; margin-bottom: 0; margin-top: 0; padding-bottom: 0; padding-top: 0; padding-left: 0;”>@SuppressWarnings(“unchecked”) // The cast is safe because…</pre>
</td>
</tr>
</tbody>
</table>******
On the other hand, avoid comments that just repeat what the code does. These are just noise:
<table class=“my-bordered-table” style=“width: 100%;”>
<tbody>
<tr>
<td style=“background-color: #f4cccc; vertical-align: top; width: 50%;”>
<pre style=“background-color: #f4cccc; border: 0px; color: black; margin-bottom: 0; margin-top: 0; padding-bottom: 0; padding-top: 0; padding-left: 0;”>// Get all users.
userService.getAllUsers();</pre></td>
<td style=“background-color: #f4cccc; vertical-align: top; width: 50%;”>
<pre style=“background-color: #f4cccc; border: 0px; color: black; margin-bottom: 0; margin-top: 0; padding-bottom: 0; padding-top: 0; padding-left: 0;”>// Check if the name is empty.
if (name.isEmpty()) { … }</pre></td>
</tr>
</tbody>
</table>
-