🔧

ラーメンオーダー自動生成装置に対するコードレビュー / コードレビューの雰囲気再び

2023/01/20に公開約7,900字

はじめに

「図月つくる[1]」というYouTuberをご存じでしょうか。私は存じ上げています。
その図月つくるさんが「ラーメンオーダー自動生成装置」というラーメン二郎[2]のオーダー(通称「ニンニク入れますか?」コール)をランダム生成する装置を開発されました。
その装置の開発に当たり、組込みソフトウェアの開発を行われているのですが、そのソースコードが公開されています。
今回はそのソースコードを用いて、コードレビューの雰囲気を味わう会第2回[3]を行いたいと思います。

具体的には下記のような進行となります。

  1. 装置の仕様説明
  2. 装置のために実装されたコードの紹介
  3. 上記コードに対する私のコードレビューを公開

装置の仕様

本記事で取り扱う装置およびソースコードの出典はこちらの動画[4]になります。

本記事で取り扱う装置は下記の手順で動作します。

  1. スイッチを押す
  2. スイッチの入力を受け、Arduino[5]を用いて下記の手順で5つのモータを制御する
    1. すべてのモータを起動しロールを回転させる
    2. 上から順番に下記の手順でモータを制御する
      1. ランダムで2~5秒待つ
      2. 対象のモータを停止させロールを停止する

以上が本記事で取り扱う装置の仕様です。
今回コードレビューの対象とするのは、ステップ2で使用するArduinoのソースコードです。

今回のコードレビュー対象となるソースコード

//スイッチ指定
const int button = 2;
//モータドライバ1-1
const int IN1 = 3;
const int IN2 = 4;
//モータドライバ1-2
const int IN3 = 5;
const int IN4 = 6;
//モータドライバ2-1
const int IN5 = 7;
const int IN6 = 8;
//モータドライバ2-2
const int IN7 = 9;
const int IN8 = 10;
//モータドライバ3-1
const int IN9 = 11;
const int IN10 = 12;

void setup() {
  // put your setup code here, to run once:
  pinMode(button, INPUT_PULLUP);
  pinMode(IN1, OUTPUT);
  pinMode(IN2, OUTPUT);
  pinMode(IN3, OUTPUT);
  pinMode(IN4, OUTPUT);
  pinMode(IN5, OUTPUT);
  pinMode(IN6, OUTPUT);
  pinMode(IN7, OUTPUT);
  pinMode(IN8, OUTPUT);
  pinMode(IN9, OUTPUT);
  pinMode(IN10, OUTPUT);
  Serial.begin(9600);
}

void loop() {
  // put your main code here, to run repeatedly:
  boolean value;
  value = digitalRead(button);
  Serial.println(value);
  //ボタンを押した時、全モータ動作
  if (value == 0) {
    digitalWrite(IN1, HIGH);
    digitalWrite(IN2, LOW);
    digitalWrite(IN3, HIGH);
    digitalWrite(IN4, LOW);
    digitalWrite(IN5, HIGH);
    digitalWrite(IN6, LOW);
    digitalWrite(IN7, HIGH);
    digitalWrite(IN8, LOW);
    digitalWrite(IN9, HIGH);
    digitalWrite(IN10, LOW);
    //2~5秒後に順にモータ停止
    delay(random(2000, 5000));
    digitalWrite(IN1, LOW);
    digitalWrite(IN2, LOW);
    delay(random(2000, 5000));
    digitalWrite(IN3, LOW);
    digitalWrite(IN4, LOW);
    delay(random(2000, 5000));
    digitalWrite(IN5, LOW);
    digitalWrite(IN6, LOW);
    delay(random(2000, 5000));
    digitalWrite(IN7, LOW);
    digitalWrite(IN8, LOW);
    delay(random(2000, 5000));
    digitalWrite(IN9, LOW);
    digitalWrite(IN10, LOW);
    delay(random(2000, 5000));
  }
  delay(100);
}

コードレビュー

前提として、今回のレビュー対象のコードは仕様通りの動作をするため、最低限の品質をクリアしています。
そのため、レビューはソースコードのメンテナンス性向上を狙う内容となります。

繰り返し記述されている処理をループにすべき

本件のコードが将来的にどのように拡張されるか、つまりこの装置がどのように発展し得るかを考えた場合、真っ先に想起されるのはモータの数の増減です。
そのため、本件のコードの修正方針も、モータの数の増減に関する対応の容易性を実現するものとなります。

これを実現する方法にはいくつかの手法が考えられますが、最も手軽な実現方法はピン番号を配列で管理し、処理をループで記述する方法です。

    //2~5秒後に順にモータ停止
    delay(random(2000, 5000));
    digitalWrite(IN1, LOW);
    digitalWrite(IN2, LOW);
    delay(random(2000, 5000));
    digitalWrite(IN3, LOW);
    digitalWrite(IN4, LOW);
    delay(random(2000, 5000));
    digitalWrite(IN5, LOW);
    digitalWrite(IN6, LOW);
    delay(random(2000, 5000));
    digitalWrite(IN7, LOW);
    digitalWrite(IN8, LOW);
    delay(random(2000, 5000));
    digitalWrite(IN9, LOW);
    digitalWrite(IN10, LOW);

例えば、上記のモータを止める処理ですが、ピン番号違いで同じ処理を5回繰り返しています。
このようなケースでは配列(array[6])とループ(for[7])を組み合わせると簡潔に記述できます。

    //2~5秒後に順にモータ停止
    for(int i = 0; i < MD_NUMS; i++) {
      delay(random(2000, 5000));
      digitalWrite(MDS[i][0], LOW);
      digitalWrite(MDS[i][1], LOW);
    }

上記のコードで、MD_NUMSはモータの総数を表す定数、MDSはモータのピンのペアを格納した配列です。
例えばMDS[0][0]は1番目のモータの奇数番ピンを表し、MDS[2][1]は3番目のモータの偶数番ピンを表します。

MD_NUMSおよびMDS配列は、例えば下記のように定義をします。

//スイッチ指定
const int button = 2;
//モータドライバ1-1
const int IN1 = 3;
const int IN2 = 4;
//モータドライバ1-2
const int IN3 = 5;
const int IN4 = 6;
//モータドライバ2-1
const int IN5 = 7;
const int IN6 = 8;
//モータドライバ2-2
const int IN7 = 9;
const int IN8 = 10;
//モータドライバ3-1
const int IN9 = 11;
const int IN10 = 12;

//モータドライバの総数
const int MD_NUMS = 5;
//モータドライバのピン番号ペアの配列
const int MDS[MD_NUMS][2] = {{IN1, IN2}
                            ,{IN3, IN4}
			    ,{IN5, IN6}
			    ,{IN7, IN8}
			    ,{IN9, IN10}
			    };
配列についての詳細

配列とは通し番号(index)が振られた一連の変数です。
例えば、int a[3];と宣言した場合、下記のような3つの箱を持つ配列が生成されます。

0 1 2
0 0 0

Arduinoの配列の通し番号は0から始まるため、1番目を参照する場合は0、2番目を参照する場合は1、3番目を参照する場合は2、のように指定します。

int a[3];
a[0] = 1;
a[1] = 2;
a[2] = 3;

この処理の結果、下記のようになります。

0 1 2
1 2 3

値を取り出すこともできます。

int v = a[2][0]; // v = 3となる

配列の宣言と同時に値をセットすることもできます。

int a[3] = {1, 2, 3};

配列の通し番号には変数が使用できるため、ループと組み合わせることで便利に通し番号を指定できます。

for (int i = 0; i < 3; i++) {
  Serial.println(a[i]);
}

また、配列の箱の個数指定には定数を用いることもできます。
下記のコードでは、3つの箱を持つ配列が生成されます。

const int SIZE = 3;
int a[SIZE];

さらに、配列は入れ子にすることができます(多次元配列と呼びます)
特に2重の入れ子になった配列を二次元配列と呼びます。
例えば、int a[3][2];と宣言した場合、下記のような配列が生成されます。

0 1 2
0 0 0 0
1 0 0 0

二次元配列も同様に通し番号は0から始まります。

int a[3][2];
a[0][0] = 1;
a[0][1] = 2;
a[1][0] = 3;
a[1][1] = 4;
a[2][0] = 5;
a[2][1] = 6;

この処理の結果、下記のようになります。

0 1 2
0 1 3 5
1 2 4 6

これらのデータに参照すると下記のようになります。
2つの通し番号を指定しますが、ちょうど上記のテーブルにおける横の通し番号と縦の通し番号に相当します。

int u = a[0][1]; // u = 2となる
int v = a[2][0]; // v = 5となる

2次元配列でも宣言と同時に値をセットすることもできます。

int a[3][2] = {{1, 2}
              ,{3, 4}
	      ,{5, 6}
	      };

2次元配列でも通し番号に変数を使用できます。

for (int i = 0; i < 3; i++) {
  for (int j = 0; j < 2; j++) {
    Serial.println(a[i][j]);
  }
}

本レビューで提案したコードは、2次元配列を使用すること、箱の個数を定数で宣言すること、宣言と同時に値をセットすることの合わせ技となっています。

前述のように実装することで、例えばモータの数が1つ増えた際に下記の2ヶ所の変更で対応することが可能となります。

  • MD_NUMS を6に変更する
  • MDS 配列の末尾に対応するピン番号のペアを追加する

上記に加え、ソースの記述例は示しませんが、下記の個所でも同様の変更を行うことでモータ数の変更に強いコードにすることが可能です。

  • setup 関数内のピンの初期化処理
  • loop 関数内のモータを回す処理

あえて指摘しなかった点

  • 命名について
    • 各変数、定数の命名には議論の余地がある
    • コード量や文化を考慮すると、積極的に議論した場合のメリットが薄いと感じたため割愛
  • モータ起動/停止処理の関数化
    • 意図がより読み取りやすくなる
    • 今回の指摘点を反映したあとのコードでは、関数化をしなくても可読性は悪くないと判断し保留
    • 今後の拡張方針によっては取り組むべき課題になり得る
  • その他軽微な指摘
    • button定数もIN1などと統一するため大文字での記述が望ましい
    • digitalRead(button);の戻り値の比較時にマジックナンバーを使用しているため、定数に置き換える
      • pinMode(button, INPUT_PULLUP);と指定しているため[8]スイッチONはLOWであり、これと比較することが望ましい
    • delay値などのマジックナンバーの定数化

結論

今回コードレビュー体験会第2回を実施しましたが、レビューを行う中で驚きの連続でした。
第1回での指摘内容が解消されており、本プログラム作成者の技術力向上を強く感じ取れるものでした。
それ故に、さらに良いコードにするためにはどのような方針でレビューをすればよいかを考えるのが難しくもあり、楽しくもあり、やりがいもありました。

今回のレビュー内容を通じて、より簡潔な実装のためにはどうすればよいのか、という観点の大切さを持ち帰って頂ければ幸いです。

最後に。

  • コードレビューでソースコードの品質を向上していこう
  • コードレビューでは愛を持って全力で殴り合おう

謝辞

本記事を作成するに当たり、下記の方々が作成された情報を参照いたしました(敬称略)
謹んで御礼を申し上げます。

  • 図月つくる, ライヴラリ所属, 株式会社ゆにクリエイト

本記事を作成するに当たり、下記の製品について取り扱いました。
謹んで御礼を申し上げます。

  • Arduino, Arduino Foundation および Arduino Holding

本記事を作成するに当たり、下記の方々にご助言、ご支援を頂きました。
謹んで御礼を申し上げます。

  • 助手くん, 図月つくるさんのファンの皆様(個別のお名前は差し控えさせて頂きます)
脚注
  1. 図月つくるさんのTwitterアカウント YouTubeチャンネル ↩︎

  2. ラーメン二郎 ↩︎

  3. 前回記事はこちら ↩︎

  4. ソースコードの出典動画 ↩︎

  5. Arduinoの公式ページ ↩︎

  6. array ↩︎

  7. for ↩︎

  8. INPUT_PULLUPのレスポンス仕様 ↩︎

Discussion

ログインするとコメントできます