QA的コードレビューのススメ
こんにちは。かいりです。
普段何かと「開発経験があるQAエンジニア」を自称するわりにそれっぽいことをしたことがなかったなと思ったので、今回記事を書いてみることにしました。
少しでも参考になれば幸いです。
コードレビューとは
基本的にエンジニアが開発のためにコードを書いた時には、エンジニア内でコードレビューを行います。
このコードレビューは、
プロジェクト内のコーディングルールを守っているかを確認する
より良い記述方法がないかを検討し指摘する
バグを見つける
などを目的として行われます。
この時によく使われるのがGithubやGitLabの「プルリクエスト」です。
QAがコードに対してできること
コードレビューを完璧にしようと思うと相当ハードルは高いです。言語の理解の他にそのプロジェクトごとのコードのクセやルールなどもありますから、それらを把握した上でコードに指摘をしていくことは決して簡単ではありません。
ではQAはコードに対して何もできないのかというと、それは違うと思っています。今しがたプロジェクトごとのコードのクセやルールがあると書きましたが、そうは言っても各言語ほぼ同じような処理の書き方をするところもあります。
そういったところから、今回はQAがアプローチできそうな箇所をいくつかピックアップしていきたいと思います。
QA的レビューのポイント
例は各言語で比較して見られるように、
Java
PHP
Ruby
JavaScript
で書いていきたいと思います。
いずれもpaiza.ioで試せるようなコードにしてある（つもり）なのでコピペしていろいろいじってみると楽しそうです。
1. if文には仕様を照らし合わせる
if文とは、多くのプログラミング言語に用意されている、命令の流れに条件分岐を設定するための構文。記述した条件が満たされた場合と満たされない場合の二つに処理の流れを分岐させ、それぞれ異なる処理を実行させることができる。
つまり分岐処理のことです。簡単に例題を考えてみます。
「雨の日は傘を持ち、それ以外の天気の場合は傘を持たない」
という処理を考えた場合、各言語で書くと以下のようになります。
Java
import java.util.*;
public class Main {
public static void main(String[] args) throws Exception {
String weather = "rainy";
if (weather == "rainy") {
System.out.println("傘を持つ");
} else {
System.out.println("傘を持たない");
}
}
}PHP
<?php
$weather = "rainy";
if ($weather === "rainy") {
print "傘を持つ";
} else {
print "傘を持たない";
}
?>Ruby
weather = "rainy"
if weather == "rainy"
p "傘を持つ"
else
p "傘を持たない"
endJavascript
var weather = "rainy";
if (weather == "rainy") {
console.log("傘を持つ")
} else {
console.log("傘を持たない")
}ただ、コードを読まずとも違和感を持たれた方もいらっしゃるのではないでしょうか。
「傘を持つのは雨の日だけではなくない？」
他にもあるかもしれませんが、雪の日も傘を差す人はいると思いますし、仮にこの「雨」というステータスが「小雨」「雨」を指すものだった場合、「雷雨」などがそこに含まれないこともあります。その場合、通常の雨の日には傘を差すのに、雷雨の中傘を差さない人が爆誕します。
そして違和感のある状況が生まれてしまったらそれは「仕様バグ」の可能性が高いです。
if文には多く仕様の分岐が含まれています。if文を見かけたらその内容を覗いてみて、仕様が漏れていないか確認してみるのはいかがでしょうか。
2. 等号不等号には境界値分析
以上（A≧B）
以下（A≦B）
未満（A<B）
〜より大きい（A>B）
というワードに過剰反応してしまうQAは少なくないではないかと思っているのですがいかがでしょうか？ちなみに私は以上と聞くと「より大きいではなくて？以上？」と条件反射で聞いてしまう体質です。
この等号不等号はそのままコードで表せます。
上の例をコードに置き換えてみます。
以上（A≧B）→ A >= B
以下（A≦B）→ A <= B
未満（A<B）→ A < B
〜より大きい（A>B）→ A > B
全角記号の「≧」「≦」の代わりに「>=」「<=」が使用されているだけで、基本的には変わらないのがわかると思います。私の知る限りこれ以外の記法を使っている言語は見たことがありません。（あったらごめんなさい）
ここで、よく境界値分析にありがちな料金表の例題を出してみます。
一般は800円
65歳以上は400円
中学生（12歳〜15歳）は400円
0歳〜6歳は無料
これを整理してみると、
0歳未満はエラーにする
0歳以上、6歳以下は0円を返す
12歳以上15歳以下、もしくは65歳以上は400円を返す
上記以外は800円を返す
こんな感じでしょうか。実際にプログラムに起こすと以下のようになります。言語はJavaです。
import java.util.*;
public class Main {
public static void main(String[] args) throws Exception {
int age = 15; // 判定に使う年齢
int price = 0; // 料金
if (age < 0) {
throw new Exception("年齢は0以上の数字を入力してください");
} else if (age >= 0 && age <= 6) {
price = 0;
} else if ((age >= 12 && age <= 15) || age > 65) {
price = 400;
} else {
price = 800;
}
System.out.println(price);
}
}書いてあることは例題の日本語と同じです。ただし、一箇所私はミスをしました。65歳以上がage > 65（65歳より上）になっています。実際こういったミスは珍しいことではありません。
QAは人よりこう言った分岐に敏感だと思うので、等号不等号（+if文）を見かけたら境界値分析をしてみるのはいかがでしょうか。
なお、コードを他の言語で書くと以下の通りになります。
PHP
<?php
$age = 15; // 判定に使う年齢
$price = 0; // 料金
if ($age < 0) {
throw new Exception("年齢は0以上の数字を入力してください");
} else if ($age >= 0 && $age <= 6) {
$price = 0;
} else if (($age >= 12 && $age <= 15) || $age > 65) {
$price = 400;
} else {
$price = 800;
}
print $price;
?>Ruby
age = 15 # 判定に使う年齢
price = 0 # 料金
if (age < 0)
raise StandardError.new("年齢は0以上の数字を入力してください")
elsif (age >= 0 && age <= 6)
price = 0
elsif ((age >= 12 && age <= 15) || age > 65)
price = 400
else
price = 800
end
p priceJavascript
var age = 15; // 判定に使う年齢
var price = 0; // 料金
if (age < 0) {
throw "年齢は0以上の数字を入力してください";
} else if (age >= 0 && age <= 6) {
price = 0;
} else if ((age >= 12 && age <= 15) || age > 65) {
price = 400;
} else {
price = 800;
}
console.log(price)さて、ここからは比較的テクニカルな用語が多く登場します。ご興味のある方はお付き合い頂けると嬉しいです。
3. nullにご注意
「ぬるぽ」「ｶﾞｯ」
というやりとりはもはや今となっては通じないものなのかもしれませんが、かつて2chで流行っていたこのやりとりの元ネタはJavaで発生する例外の一つ「NullPointerExeption」です。
まずnullって何？という話ですが要は「何のデータも含まれない状態のこと」です。
プログラミング初心者がつまづきやすい、0とnullの違いを解説した画像です。 pic.twitter.com/I2JiXO8OUr
— R SATO（佐藤 玲） (@raysato) February 20, 2017
有名なトイレットペーパーの例。
トイレットペーパーの残量を数字で例えると左は空になっているので0、右はそもそも芯も存在していないのでnull、となります。
このとき、たとえば「トイレットペーパーの残量を出力する」処理を考えてみましょう。左のトイレットペーパーは「0cm」です。この場合、処理はエラーにはなりません。
ですが、右の「null」のトイレットペーパーの残量を出そうとしても…そもそもトイレットペーパーの概念そのものがありませんから、どうにもできないわけです。ここで処理はエラーになります。
これを実際にコードに起こしてみます。ここではせっかくなのでJavaを使います。
import java.util.*;
public class Main {
public static void main(String[] args) throws Exception {
Integer toiletPaper = 0;
System.out.println(toiletPaper.toString() + "cm");
}
}正常に値が入っていると結果は以下の通り。
0cmちゃんと入れた通りの値が返ってきました。
ではここでnullを入れるとどうなるでしょう。
import java.util.*;
public class Main {
public static void main(String[] args) throws Exception {
Integer toiletPaper = null;
System.out.println(toiletPaper.toString() + "cm");
}
}実行結果は以下。
Exception in thread "main" java.lang.NullPointerException: Cannot invoke "java.lang.Integer.toString()" because "<local1>" is null
at Main.main(Main.java:6)ぬるぽになりました。
これがどう実際のテストに影響してくるかというと、WEBアプリで言うならフォームの入力が代表例になります。
「特定のフォームを空で入力したらエラーになった」という事象に遭遇したことのある方がどれくらいいらっしゃるかは不明ですが、少なくとも私はQA中に何度か遭遇したことがあります。
上の例で言えばトイレットペーパーの値にフォームから送られてきた長さが入るわけですが、この時にnullが入る、というとイメージしやすいでしょうか？
気にすべきは「nullに対しての何かしらの処理」がエラーになることが多いということです。
値がnullになりうるかそうじゃないか、nullになりうる場合はぬるぽになるような処理を呼び出していないかを気にすべきだと思います。
ちなみに他の言語の例。ここからは処理と実行結果をまとめて書いていきます。まずはPHPから。
[処理]（数字が入るパターン）
<?php
$toilet_paper = 0;
print strval($toilet_paper)."cm";
?>
[結果]
0cm
[処理]（null）
<?php
$toilet_paper = null;
print strval($toilet_paper)."cm";
?>
[結果]
cmあれ？エラーにならないじゃん！と思うと思いますがここにもバグな発生しがちな部分が含まれています。一旦気にしないでいきましょう。次はRubyです。
[処理]（数字が入るパターン）
toilet_paper = 1;
p toilet_paper.to_s + "cm"
[結果]
"1cm"
[処理]（null）
toilet_paper = nil;
p toilet_paper.to_s + "cm"
[結果]
"cm"こちらもエラーになりませんでした。が、内容はPHPと同じです。法則性が見えてきたでしょうか。最後にJavascriptです。
[処理]（数字が入るパターン）
var toilet_paper = 0;
console.log(toilet_paper.toString() + "cm");
[結果]
1cm
[処理]（null）
var toilet_paper = null;
console.log(toilet_paper.toString() + "cm");
[結果]
TypeError: Cannot read properties of null (reading 'toString')
at Object.<anonymous> (/Users/user/PhpstormProjects/atCoder/test.js:2:26)
at Module._compile (node:internal/modules/cjs/loader:1246:14)
at Module._extensions..js (node:internal/modules/cjs/loader:1300:10)
at Module.load (node:internal/modules/cjs/loader:1103:32)
at Module._load (node:internal/modules/cjs/loader:942:12)
at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:83:12)
at node:internal/main/run_main_module:23:47エラーになりました。NullPointerExceptionではないですが、nullをtoStringできないよ、と言ってます。
4. 型変換にもご注意
先ほど出てきたPHPとRubyでエラーにならなかったことに繋がります。
型というのは要するにその変数に入る値のタイプです。
種類はいろいろありますが、今回使っているのは以下の上二つです。三つ目はこれから使います。
integer（intとも。数字の型。0と表記する）
string（文字列の型。"cm"と表記する）
boolean（boolとも。true, falseのどちらかだけが入る）
先ほどのトイレットペーパーを数字で「0」と入れて、それを文字列型に変換して"cm"とくっつけて表示する、というのが今回の処理でした。
この変換のことを一般的に「型変換」といいます。
JavaとJavascriptではnullを文字列変換しようとするとエラーになりました。"null"とはなりません。
一方、RubyとPHPの結果を見てみましょう。
cmこれがどういう意味になるのかというと、つまり「nullを暗黙的に空文字（""のこと）に変換する」ということです。
これだけ聞くと、「それ、何かまずいの？」と思われるかもしれません。別にこれだけだとまずくありません。たぶん。が、これを発展させるといろいろ面倒くさいことになります。
以下は、PHPでさまざまなものをboolに型変換した例です。
bool に変換する場合、次の値は false とみなされます。
boolean の false
integer の 0 (ゼロ)
float の 0.0 および -0.0 (ゼロ)
空の文字列 ""、 および文字列の "0"
要素の数がゼロである 配列
unit 型 NULL (値がセットされていない変数を含む)
bool 型へキャストするように動作がオーバーロードされた内部オブジェクト。 例: 属性がない空要素から作成された SimpleXML オブジェクト。
はい？と思うかもしれませんが、引用続きます。
<?php
var_dump((bool) ""); // bool(false)
var_dump((bool) "0"); // bool(false)
var_dump((bool) 1); // bool(true)
var_dump((bool) -2); // bool(true)
var_dump((bool) "foo"); // bool(true)
var_dump((bool) 2.3e5); // bool(true)
var_dump((bool) array(12)); // bool(true)
var_dump((bool) array()); // bool(false)
var_dump((bool) "false"); // bool(true)
?>見覚えのない型がありますがそれは一旦無視してstringとintegerにフォーカスを当てると、
数字の1はtrue（まあわかる）
数字の0はfalse（わかる）
数字の-2はtrue（・・・ふーん？）
文字列の"false"はtrue（なるほど）
文字列の"0"はfalse（？？？？？？？）
開発する側も必ずしもこのような暗黙的な処理を覚えているわけではありません。なので、こういった型変換にはバグが付き物です。
型変換している箇所を見つけたら注意するようにしましょう。
各言語の型変換（明示的に行うものをキャストと言います）の書き方を以下に記載しておきます。int→stringの例を書きます。
[Java]
String str1 = "100";
int num1 = Integer.parseInt(str1);
[PHP]
string str1 = "100";
int num1 = (int）str1;
[Ruby]
str1 = "100"
num1 = str1.to_i
[Javascript]
var str1 = "100";
num1 = Number(str1)上記の通り言語によって書き方はまちまちですが、大体の言語には型変換の処理があることがわかると思います。ここにない言語を使用している、という場合でも「○○（その言語） 型変換」など検索すれば出てくる場合が多いです。
終わりに
いかがでしたでしょうか。個人的に思うよくあるバグ（結構大きめ）の発生箇所のパターンをいくつか挙げてみました。
他にもこんなのがあるよ、などあればご紹介いただけると嬉しいです。
ここまでお読み頂きありがとうございました。
この記事が気に入ったらサポートをしてみませんか？