自主的20%るぅる

各々が自主的に好き勝手書くゆるふわ会社ブログ
自主的20%るぅる > 最新記事 > 技術ネタ > ほかのひとのプログラムを読むときに気をつけていること
技術ネタ

ほかのひとのプログラムを読むときに気をつけていること

この業界でお仕事をしていると、自分以外の人が書いたプログラムを読む機会がよくあります。

仕事などでほかの人のプログラムを読んでみると、
「自分が担当したプログラムはサクサク読めるんだけど、ほかの人が書いたプログラムは処理を追うのが辛い・・・。なんじゃこりゃ？」
なんてことを考えてしまうこともよくあります。

それでも人生の半分くらいをプログラミングをして過ごしてきたので、
自分なりにほかのひとが書いたプログラムを読むときに気を付けていることがわかってきました。

ちょっとそのあたりのコツなんかを書いてみようと思います。

プログラムを読めなかった若かりし頃の苦い思い出

新しい職場に入ったとき「準備が間に合ってないので、とりあえず既存のプログラムを読んでてください ＞＜」なんてことを言われることもあると思います。

若かりし頃の私は、この「プログラムを読む」ということが具体的には何をすればよいのかが良くわかっていませんでした。

とりあえずあてもなく適当なファイルを開いては「うーむ、ようわからん」と思いながらほかのソースファイルを開いてまた繰り返す という時間の使い方をしてしまい、いざ業務が始まってから結局わからないことだらけで周りの人に迷惑をかけてしまう・・・ということがよくありました。1)若かりし頃の苦い思い出ですねぇ・・・

いまになって思い返してみると、当時の私はプログラムを読むことを本や文書を読むのと同じように考えていたようです。

しかし実際には、プログラムを読むときには本や文書を読むのとは違うことを意識しないとうまくいかないことがわかってきました。

まずはどの部分の処理を追うのかを決める

若かりし頃の私が失敗してしまった一番の原因はどの処理を追う2)この処理がどうやって動いているのかを解析する みたいな意味合いですのかを明確にしていなかったことです。

どの処理を追うのかが明確になっていないと、処理の解析を進めることがそもそもできません。
おそらく若かりしの私のように、適当にソースファイルをぱらぱら開いていくくらいしかやりようがないと思います・・・。

もし自分が担当する機能や手を入れたい機能が明確になっているのであれば、その機能がどう動いているのかから見てみるようにしてみましょう。

もしどこを担当するかが明確になっていないようであれば、周りのひとにどの機能を見ればシステム全体のクセや思想がわかりやすいのかを聞いてみるのがおススメです。

周りに聞ける人がいないようであれば、とりあえず気になった機能を見ていくのがいいかもしれません。3)というかそれしかやりようがないですよね・・・

正直どの機能でもいいので、とりあえず解析する機能をどれにするかを決定することが大切です。

次にどのレベルまで解析するかを決める

解析する機能を決めたら、次はどこまで解析するかを決めていきます。

私がソースを追うときには、ざっくり以下のレベルで解析することが多いです。

  • システム全体で共通するつくりや使い方を解析する
  • ある機能がどういう順序で処理をしているのかを解析する
  • ある処理がどんな実装をされているのかを解析する

必要に応じてどのレベルまで解析するのかは異なってきます。

どのレベルまで解析するかを決めておくと、見なくていいところや考慮しなければいけないことが明確になり、処理を追っていくのがよりスムーズになります。

システム全体で共通するつくりや使い方を解析するコツ

はじめて触るシステムのプログラムを読むときなどはこのレベルの解析を行うのが良いと思います。

このレベルで解析するときは、まずそのプログラムが使っているフレームワークやライブラリの情報を集めましょう。

私が気にしているポイントは以下の通りです

  • どんな仕組み4)MVC・MVVC・イベントドリブン・RTOS・ORMの可否などを持ったフレームワークを使っているのか？
  • フレームワークをどのくらいカスタマイズしているか？

とりあえずどんなフレームワークを使っているのかがわかれば、google先生に聞いたりや内部資料を探すことで解析を進めていくことができます。

そのフレームワークはどんな思想で作られているのかも大事な情報です。

例えばそのフレームワークでMVCを採用している場合

  • 画面周りはView関連のファイルを見ればよさそう
  • 処理を追うときにはとりあえず関係しそうなControllerからみていけばよさそう
  • DBへのアクセスはModel関連のファイルを見ればよさそう
  • フレームワークが用意しているBaseクラスがあるので使えそうな共通処理が見つかるかもしれない
  • 汎用処理としてHelperやComponentみたいな要素もありそう

ということが予想できます。

この場合、とりあえずView・Controller・Modelはそれぞれどこに格納されているのかを確認していけば処理を追うとっかかりになりそうです。

そのフレームワークをどのくらいカスタマイズしているのかを把握できれば、
どこまでは一般的なフレームワークと同じようなことをしていて、どこから独自の処理をしているのかがわかるようになります。

一般的なフレームワークと同じところは深く追わなくても処理の流れをイメージできると思いますので、
独自の処理をしている部分を注意しておけば全体の処理の流れを把握しやすくなります。

ある機能がどういう順序で処理をしているのかを解析するコツ

実際にある機能を改修したり新規作成するときにはこのレベルで解析するのが良いと思います。

このレベルでの解析では以下のポイントを気にしています

  • 基本的に処理の詳細は深く追わない5)どんな変数があってそこにどんな値が入るか みたいなことは気にしないようにします
  • どんな順序でどんな処理(メソッドや関数)を実行しているかに着目
  • 条件分岐や繰り返し回数は必要なところだけ詳細をみる

といわれてもイメージしづらいと思いますので、例を出してみます。

実例

HogeControllerクラスに処理を追加することを想像してみます。

https://なんやかんやサービス/Hoge/hoge というURLにアクセスしたらhogeActionが実行されるものとします。
今回はこのhogeActionに処理を追加することを想像します。

※ ->や’などはエスケープされてしまうので全角文字で記載しています。ご了承ください。

class HogeController
{
    protected session;
    protected acl_info;

    public function hogeAction()
    {
        // セッションをスタート
        $thisー＞session = new Session();
        $thisー＞sessionー＞start();

        $thisー＞loginCheck();

        // ACL情報を読み込む
        $thisー＞acl_info = $thisー＞getAclInfo();

        $thisー＞execute();
    }

    protected function loginCheck()
    {
        if (!checkLoginStatus($thisー＞session)) {
            // ログインしてなかったらログインページへ飛ばす
            redirect(’login’);
        }
    }

    protected function getAclInfo()
    {
        return AclManager::getAclInfo();
    }

    protected function execute()
    {
        if (!$thisー＞acl_infoー＞hasAcl(’hoge’)) {
            // 権限が無かったらエラーページへ飛ばす
            redirect(’error’);
        }

        $input_data = $thisー＞sessionー＞get(’input_data’);
        $model = new HogeModel();
        foreach ($input_data as $data) {
            if ($data[’enable’]) {
                // 有効データを保存する
                $modelー＞save($data);
            }
        }
    }
}

一番最初に見るのは、publicなメソッドであるhogeActionです。

このメソッドはおおまかにいうと

  1. セッションを開始する
  2. ログインチェックをしている
  3. 権限情報を取得している
  4. executeを実行している

という4つの処理で構成されています。

今回新しく処理を追加することを考えると、1.～3.の処理は深く追わなくてもよさそうです。

続いてexecuteメソッドを見てみると、

  1. 権限確認
  2. セッションから情報を取得
  3. セッションから取得したデータを保存する

という3つの処理を行っていることがわかります。

処理を追加する観点で考えると、このexecuteメソッドの3.前後に処理を追加するのがスムーズそうだなぁという想像ができます。

メソッドやデータなどが混みあってくるともっといろんなメソッドを追っていく必要があるかもしれませんが、
それでも基本的にはこのくらいの粒度で理解していくのが良いと思います。6)この粒度でコメントが書かれていると見やすいなぁと思うことが多いです

ついどうやってログインチェックしてるんだろう？とかセッションにはどんなデータがあるの？とかどんなACLがあるの？みたいなポイントを追いたくなってしまいます。
しかし経験上そこまでの詳細を追いかけてしまうと、時間をかけて調べたけどあまり解析が進まなかった という事態に陥ることが多いです。

ある処理がどんな実装をされているのかを解析するコツ

何か不可解なバグが発生したときにはこのレベルで解析するのが良いと思います。

この場合に私が気をつけているポイントは以下になります

  • それぞれの変数はどんな役割をしていてどんな値を格納しているのか
  • 条件分岐では何を見ていてどう分岐しているのか7)変数〇〇を見て××をしている ではなく 有効データだったら××をする のように考えるイメージ
  • 繰り返す処理では何回繰り返しているのか8)変数〇〇が×になるまで ではなく データの個数分実行する のように考えるイメージ

この中でも、一番上の変数については特に気をつけてみています。

実例

今回も実例を出してみます。

ログを出力する処理にバグが見つかったことを想定します。バグの内容は

ログとして出力された文字列のタイムスタンプ部分によくわからない文字が混入している

というものです。

※ ->や’、”などはエスケープされてしまうので全角文字で記載しています。ご了承ください。

class Logger
{
    protected $file_name;
    protected $file;

    public function __construct($file_name)
    {
        $thisー＞file_name = $file_name;
        $thisー＞file = null;
    }

    public function __destruct()
    {
        $thisー＞closeFile();
    }

    public function writeLog($message)
    {
        if ($message === ’’) {
            // メッセージが空なら何もしない
            return true;
        }

        $write_message = $thisー＞getTimeStamp() . $message . ”\n”;

        try{
            $thisー＞openFile();
            $result = fwrite($thisー＞file, $write_message);
            $thisー＞closeFile();
        } catch(\Exception $e) {
            throw new \Exception(’writeLog error. ’ . $eー＞getMessage() . ”\n” . $eー＞getTraceAsString());
        }

        return $result;
    }


    protected function openFile()
    {
        $thisー＞file = fopen($thisー＞getFilePath(), ’a’);
    }

    protected function closeFile()
    {
        if ($thisー＞file !== null) {
            fclose($thisー＞file);
            $thisー＞file = null;
        }
    }

    protected function getTimeStamp()
    {
        return date(’[Y-m-d H:i:S]’);
    }

    protected function getFilePath()
    {
        return ’/tmp/logs/’ . $thisー＞getFileName();
    }

    protected function getFileName()
    {
        $today = date(’Y-m-d’);
        return $thisー＞file_name . ’_’ . $today . ’.log’;
    }
}

// 使っている箇所
$logger = new Logger(’loglog’);

$loggerー＞writeLog(’なんかよからぬことが起こったようです’);

まずはLoggerクラスのコンストラクタとデストラクタを見てみます。

コンストラクタではファイル名の設定とファイルオブジェクトの初期化を行っているようです。
デストラクタではログファイルのクローズ処理を行っているだけのようです。

これらはバグに関係なさそうなのでスルーします。

次にログを書き込むwriteLogメソッドを見ていくと、 引数として受け取った $message の前方にタイムスタンプを、後方に改行コードを結合して $write_message 変数を初期化したようです。

この変数は後ほどfwriteの第二引数として使われているので、この $write_message がログとして書き込まれる文字列であることが予想できます。

ということはこの $write_message を初期化する際に作成しているタイムスタンプに問題があるようなので、タイムスタンプを作っている getTimeStamp メソッドを見てみます。

その中身をよく見ると何かおかしなことに気づきます。9)[Y-m-d H:i:S]ではなく[Y-m-d H:i:s]が正しい設定です

おわりに

最後にまとめると

  • どの処理を解析するのかを明確にする
  • どこまでの深さまで解析するのかを明確にする

この2点を意識しておくだけでかなりプログラムを読みやすくなるのではないかと思います。

ほかにも細かいテクニックはあるのですが、あまりにも長くなりすぎるのでまた別の機会に・・・。

この情報が若かりし頃の私と同じような苦しみを味わったことがある方の助けになると幸いです。

注訳はこちら

注訳はこちら
↑1 若かりし頃の苦い思い出ですねぇ・・・
↑2 この処理がどうやって動いているのかを解析する みたいな意味合いです
↑3 というかそれしかやりようがないですよね・・・
↑4 MVC・MVVC・イベントドリブン・RTOS・ORMの可否など
↑5 どんな変数があってそこにどんな値が入るか みたいなことは気にしないようにします
↑6 この粒度でコメントが書かれていると見やすいなぁと思うことが多いです
↑7 変数〇〇を見て××をしている ではなく 有効データだったら××をする のように考えるイメージ
↑8 変数〇〇が×になるまで ではなく データの個数分実行する のように考えるイメージ
↑9 [Y-m-d H:i:S]ではなく[Y-m-d H:i:s]が正しい設定です
Let’s share this article!

{ 関連記事 }

{ この記事を書いた人 }

アバター画像
とっつぁん

気づいたら人生の半分以上プログラムを組んで生活していた技術者です。
組み込みからwebまで大体何でもやります。
管理職とかもやってます。

寝具と山椒とカレーと麻婆豆腐とワインが好きです。

記事一覧
記事一覧へ戻る