電通総研 テックブログ

電通総研が運営する技術ブログ

"脱スパゲッティコード"してプロダクトコードを書くスタートに立つまで

この記事は電通国際情報サービス Advent Calendar 2021 21日目の記事です。

はじめに

こんにちは、ISID 金融ソリューション事業部の星野将吾と申します。2年目のひよっこエンジニアです。

アドベントカレンダーに参加したい!と思ったものの、ひよっこであるのもあり詳細な技術に踏み込んだことは書けないので、今回は「"脱スパゲッティコード"してプロダクトコードを書くスタートに立つまで」というタイトルでお話していこうと思います。

今まで大学の課題や研究において個人開発でしかコードを書いてこなかった私が、プロダクトコードを書く上で解決しなければならなかった課題をここに書きます。

これからプロダクトコードを書く新社会人や学生の皆様の一助になればと思います。
この手の技術記事は初めて書くので、読者の皆様お手柔らかにどうぞ…

学生時代の私とスパゲッティコード

まず本タイトルにある「スパゲッティコード」とは何かという話ですが「実行順序や構造が複雑に入り組んでいて整理されていないプログラム」を指します。

コードをスパゲッティコードたらしめている要因を挙げてみると以下のようなものがあります。

  1. 同じ処理・似たような処理が複数箇所に書かれている。
  2. 処理構造化が適切に階層化されていない。

学生時代は自分しかそのコードに触れないため、スパゲッティコードであっても自分さえ理解できていれば問題はありません。
しかしながら、プロダクトコードを書く上でスパゲッティコードは許されません。
なぜなら、自分以外の人がそのコードを触ることになるからです。他の人にとって読みにくいコードは、コードの保守性が著しく低下します。

次の章ではスパゲッティコードを防ぐに当たって気をつけるべき観点について説明します。

脱!スパゲッティコード

スパゲッティコードするための観点は以下です。

  1. コピー&ペーストしない!とことん処理を共通化
  2. 前処理・主処理・後処理を意識した構造化プログラミングの実施

コピー&ペーストしない!とことん処理の共通化

個人で開発していると、共通化を考えるのが面倒で、自分や先輩、同僚が書いた既存コードベースからついついコピー&ペーストしてしまいませんか? 私はかなりやってました。
このコピー&ペーストをやってしまうと、1か所で済んだはずのコード修正が、3, 4箇所にも及んでしまい修正コストが増加しています。
ひとりなら3, 4箇所くらいなんとかなるかもしれません。
しかし、他の人が修正するにあたっては、どこにコピー&ペーストがされているか探す必要も出るのでプロダクトコードでは大きな問題になります。

コピー&ペーストをしたい部分を見つけたら、その部分を共通関数として切り出して使いたいところで呼びだすことができないか考えてみましょう。 もちろんコピー&ペーストをしていなくても、同じ処理を書いてしまうこともあります。まずは、コピー&ペーストを止めるところから共通化を行う意識を高めていけるとよいでしょう。

コピー&ペースト厳禁!みたいな話をしましたが、共通関数として切り出すことができなかったり、無理やり共通化するのが不適切なケースも多くあります。
しかし、この判断は個々のプロダクトにおける設計と関係する話になってくるので、本記事での説明は割愛します。
ただ、「共通化できないか?」と考える姿勢はスパゲッティコード化を防ぐ上で大変重要です。

前処理・主処理・後処理を意識した構造化プログラミングの実施

構造化プログラミングとは、簡単に言ってしまえば「処理を階層化すること」です。

まず階層化されていないコードの例を以下に示します。
言語は大学のプログラミング演習等の講義でよく扱われるであろう、C言語で書いてみます。
内容は二つの数字の加減算を行う簡単なプログラムです。

#include <stdio.h>

int main(void){
  // 前処理
  printf("加減算へようこそ。\n");
    
  // 主処理
  while(1) {
      int a, b, op, result, ctrl;
      char op_char;

      printf("1つ目の数字を入力してください\n");
      scanf("%d",&a);
    
      printf("2つ目の数字を入力してください\n");
      scanf("%d",&b);
    
      printf("加算なら0, 減算ならそれ以外を入力してください\n");
      scanf("%d", &op);
    
      if (op == 0) {
          result = a + b;
          op_char = '+';
      }else{
          result = a - b;
          op_char = '-';
      }
    
    
      printf("計算結果:%d %c %d = %d\n", a, op_char, b, result);
      printf("他の計算をしたい場合、1を入力してください\n");
      scanf("%d", &ctrl);
      if( ctrl != 1 ){
          break;
      }
  }

  // 後処理
  printf("お疲れ様でした。\n");
  return 0;
}

これは短いコードなので、一つの関数に書いても大きな問題はありません。
しかし、これ以上に長いコードや、if文やfor文などがされるコードは読みにくくなります。 読みにくいコード = 理解が困難なコードであるため、コードの保守性の悪化を招くこととなります。

このコードを構造化してみると以下のようになります。

#include <stdio.h>

void input(int *ptr_a, int *ptr_b, int *ptr_op){
  printf("1つ目の数字を入力してください\n");
  scanf("%d", ptr_a);
    
  printf("2つ目の数字を入力してください\n");
  scanf("%d", ptr_b);
    
  printf("加算なら0, 減算ならそれ以外を入力してください\n");
  scanf("%d", ptr_op);
}

int calc(int a, int b, int op){
  int result = 0;

  if (op == 0) {
    result = a + b;
  } else {
    result = a - b;
  }

  return result;
}

void output(int a, int b, int result, int op){
  char op_char;

    if (op == 0) {
      op_char = '+';
    } else {
      op_char = '-';
    }

    printf("計算結果:%d %c %d = %d\n", a, op_char, b, result);
}

int scan_continue_ctrl(void){
  int ctrl = 0;
  printf("他の計算をしたい場合、1を入力してください\n");
  scanf("%d", &ctrl);
  return ctrl;
}

void exec(void){
  while (1) {
    int a, b, op, result, ctrl = 0;

    // 前処理
    input(&a, &b, &op);
    
    // 主処理
    result = calc(a, b, op);
    
    // 後処理1
    output(a, b, result, op);

    // 後処理2
    ctrl = scan_continue_ctrl();
    if ( ctrl != 1 ) {
      break;
    }
  }
}

int main(void){
  // 前処理
  printf("加減算へようこそ。\n");
    
  // 主処理
  exec();

  // 後処理
  printf("お疲れ様でした。\n");
  return 0;
}

コード中のコメント文にあるように、多くのケースで前処理・主処理・後処理というパターンで構造化すると上手く構造化できます。
例えば、ファイルに対する書き込み処理という仕様のコードでも、

  • 前処理:ファイルのオープン
  • 主処理:ファイルへの書き込み
  • 後処理:ファイルのクローズ

のように構造化できます。

また、例のコードのexec関数に着目してみてください。exec関数はmain関数の主処理として切り出されたものですが、その中でさらに前処理・主処理・後処理と分けることができています。
ただ後処理はあえて一つにまとめませんでした。というのも、output関数とscan_continue関数をまとめたときに与える関数名がpost_processくらいしか思いつかなかったためです。
該当行周辺における関数のまとめ方次第でこの判断の是非は分かれると思います。
今回例えばpost_process関数という関数を用意して後処理をまとめてしまうと、input関数とcalc関数より抽象度が高い関数名になってしまいます。抽象度がばらつくとそれだけでソースコード理解の難易度は上昇します。

また、今回に限った話ですが、input関数の存在はoutput関数の存在を期待しています。ただ入力を受け付けるだけのプログラムならいいのですが、今回は結果を表示するまでが必要とされています。よって、output関数がないと、ぱっと見で「え?どこで出力するの?」となってしまいます。
このように、必ずしも前処理・主処理・後処理の3つにキレイに分かれるとは限りません。しかしながら、慣れないうちはこの分け方を意識すると、構造化の方針が見えやすいので非常におすすめです。

まとめ

今回この記事では、ただ動きさえすればいいスパゲッティコードを改善するための、2つの観点をお話しました。
1つ目は「共通化、2つ目は「構造化」です。
この2点に気をつけるだけでぐっとコードの保守性はあがると思います。
もちろん最初は気をつけてもなかなかうまくいかないものです。私もまだ完璧にはできていません。
たくさんコードを書いてたくさんレビューをしてもらう。これをくじけずに継続することで着実に身についていくと思います。 私も引き続きくじけずに頑張ります…

ただこの "脱スパゲッティコード" を成し遂げたらプロダクトコードとして完璧かと言われればそうではありません。
他にも変数・関数の命名の仕方、変数のスコープなど、気をつける点は多くあります。
今後、今回触れていない内容についても、投稿できればなとは思っています。(今回は上手くまとめられず…)

最後に

今回この記事が書けるようになるレベルまでマンツーマンのご指導をしていただいた、12/1執筆者の太一さん(記事)と12/17執筆者の水野さん(記事)には大変感謝しております。

さて、明日12/22のアドベントカレンダーは、山口さんによるセキュリティ周りの記事が投稿される予定です。
是非他の記事もお楽しみください!

執筆:@hoshino.shogo、レビュー:@sato.taichiShodoで執筆されました