新人プログラマーが「リーダブルコード」を読んでみた(前編)

お久しぶりです。
いきなりですが、僕は、大学時代にかなりわがままなコードを書いていました。
そんなわがままコードを矯正するために、名著「リーダブルコード」を読んだので、
その感想を書いていきます。

この本の副題は「より良いコードを書くためのシンプルで実践的なテクニック」です。
それでは、早速そのテクニックを見てみましょう!

注)
この記事は、ひたすら読書感想文が続きます。
それでも許せますよ!という人だけ読んでいただければ幸いです。

1. 理解しやすいコード

<重要ポイント>

  • コードは、他の人が見て最短時間で理解できるように書かなくてはならない。
  • 「このコードは理解しやすいだろうか?」と自問自答してから、次のコードを書く。

僕のような新人プログラマーは、コードを書くのが非常に遅いです。
なので「早く終わらさなきゃ!」という気持ちになることが多いです。
しかしそのコードは他の人が見るものなので、理解しにくいコードは他の人の時間を奪います。
もちろん、バグの温床にもなります。
他の人が最短で理解できるコードを、常に意識したいですね。

2. 名前に情報を詰め込む

<重要ポイント>

  • 目的を明快に表す単語を用いて、変数や関数の命名をする。
  • 汎用的であることに意味がある場合以外は、汎用的な命名を避ける。
  • 名前に単位などの情報を付与することで、誤解を生まない分かりやすい命名となる。

この章の内容は、意識することはできれど実践は難しいと感じました。
しかし、できるようにならなくてはいけません。
英単語の語彙を増やせるよう努力します。
オープンソースのプロジェクトを読み、
どのような命名をしているのか勉強するのも良さそうですね!

3. 誤解されない名前

<重要ポイント>

  • 名前を決定する前に、その名前を「このように誤解されないだろうか」と想像する。
  • ある単語に、「ユーザーがどのような期待を持っているか」
  • を考える。

この章は、前章で取り上げられた命名法の、さらに踏み込んだ内容を説明していました。こちらについても、常に他の命名がないかと意識することで、次第に上達していくのかなぁという感想です。

4. 美しさ

<重要ポイント>

  • 複数のコードブロックで同じような処理のとき、コードのシルエットも同じようにする。
  • コードの「列」を揃えることで、コードの概要が理解しやすくなる。
  • ある場所で決めた処理や順序は、他の場所でも同じ順序にする。
  • 空行を駆使し、論理的な段落を作る。
  • 「一貫性」のあるスタイルは、「正しい」スタイルよりも大切。

コードの美しさは非常に重要です。
見にくいコードって誰も触りたくないですよね。
そんなコードを生まないためのテクニックが、この章には詰まっていました。
中でも「一貫性」>「正しい」というのは、意識したことがなかったので、今後注意したいです。

5. コメントすべきことを知る

<重要ポイント>

  • コードからすぐに分かることはコメントに書かない。
  • コメントには、考えの過程や意見など、自分の思考を記述する。
  • 全体像を要約するようなコメントはグッド!
  • 優れたコード > 酷いコード + 優れたコメント

この章は、コードへのコメントについてです。
ここは、非常に耳が痛い章でした。
というのも、私自身、コメントマシーンのようになっていたからです。
思えば、大量のコメントは、自分のコードに自信がないということの表れだったように思います。
今後は「シンプルなコード」+「補足コメント」を意識しようと思います。

6. コメントは正確で簡潔に

<重要ポイント>

  • 曖昧な代名詞の使用を避ける。
  • 関数の正確な動作を書くべきなのか、関数の意図を書くべきなのか考える。
  • 情報密度の高い言葉を選択する。

前章に引き続き、コメントに関する章です。
こちらでは、具体的にどのようなコメントを書くのかというテクニックを紹介していました。
命名法と同じく、常に意識することや、他のコードをよく読むことが大事だと思います。

7. 制御フローを読みやすくする

<重要ポイント>

  • if/else 文では、条件文は肯定形を先に書き、否定形を後に書くようにする。
  • 肯定形が複雑になってしまう場合は、上述の限りでない。
  • 比較の条件を使うときは、変動するものを先に、安定したものを後に書く。
  • 三項演算子は、どうしても使わなければいけないというとき以外は使わない。
  • 条件のネストは、早めに値を返すなどしてできるだけ浅くする。

制御フローをどのように見やすくするかというテクニックが詰まった章です。
大学時代は、比較的複雑なフローを書く機会が多かったので、意識できている内容が多かったです。
もちろん、意識していないこともあったので、それは自分のものにしなくてはならないです。

8. 巨大な式を分割する

<重要ポイント>

  • 「説明変数」を用いて、複雑な式を簡略化する。
  • 複雑なロジックは、ド・モルガンの法則を使ってより簡潔にする。
  • 短く理解しにくいコードよりも、少し長くて明快なコードを書く。

この章では、
① exp->foo->bar
のようなメソッドに次ぐメソッドや、
② (A && !B) || (!A && C)
のような複雑なロジックをどのように簡潔に書くのかという章です。

ちなみに、①は説明変数を用いて簡略化できます。
②は ド・モルガンの法則を使うと、!A || B || A || !C となり、
A または NOT A となります。つまり、いつでも通るみたいな条件文になります。
これらのテクニックは覚えておく必要がありますが、問題は使いどころです。
その判断の目を養わなくてはならないと感じました。

まとめ

今回は、以上です。
各章で、シチュエーションごとのテクニックを紹介していましたが、
共通して伝えていることは、「そのコード大丈夫?もっと見やすくできないの?」
ということです。
この言葉を、常に意識してコードを書かなきゃですね。
残りの章については、また後日記事にしようと思います。

DBUnit 使ってみた

最近、テストコードを書いているのですが、DB 周りのテストに大変苦戦しておりました。PHPUnit を使って書いているのですが、ただの Insert 文をテストするのにもかなり遠回りしなくちゃいけないことがあるんですよね。。。
今回は、そんな PHPUnit の拡張機能である DBUnit を使ってみます。

参考サイト:
https://phpunit-tenkoma-working.readthedocs.io/ja/latest/database/
https://www.ritolab.com/entry/168
https://qiita.com/takatama/items/63c7c82108af48b7bbdb

何ができるか

以下が DBUnit の特徴です。

  • DB の初期状態を別ファイルで定義して、テスト用の初期環境を作る。
  • 上記の別ファイルデータを、データセットやデータテーブルと呼ぶ。
  • アサーションで比較に使う値もデータセットで定義可能。
  • データセットは、XML、YAML、CSV、PHP の配列等々をサポート。
  • テーブルの行数カウントや、テーブルの情報自体を比較可能。
  • テスト実行後は、(特定のメソッドを定義すれば)自動で DB の変更を消去!

偉い感じしますね!

インストール方法

今回は、Composer を使ってインストールします。
アプリケーションのディレクトリで、下記コマンドを実行します。

composer require --dev phpunit/dbunit

同ディレクトリの、composer.json に下記記述が入っているはずです。

"require-dev": {
    "phpunit/dbunit": "^4.0"
}

これで DBUnit のインストールは完了です。

DBUnit 使い方

phpunit.xml に下記の記述をします。
ここでは、テスト用 DB に接続するための定数を定義しています。

<phpunit bootstrap="..\vendor\autoload.php">
  <php>
    <ini name="display_errors" value="on"/>
    <var name="DB_DSN" value="mysql:dbname=local_test;host=localhost" />
    <var name="DB_USER" value="root" />
    <var name="DB_PASSWD" value="" />
    <var name="DB_DBNAME" value="local_test" />
  </php>
</phpunit>

次に、DB 接続するメソッドを記述した、抽象クラスを定義します。
各テストコードは、このクラスを継承することになります。

<?php
use PHPUnit\Framework\TestCase;
use PHPUnit\DbUnit\TestCaseTrait;

abstract class Generic_Tests_DatabaseTestCase extends TestCase{
    use TestCaseTrait;

    // PDO のインスタンス生成は、クリーンアップおよびフィクスチャ読み込みのときに一度だけ
    static protected $pdo = null;

    // PHPUnit\Dbunit\Database\Connection のインスタンス生成は、テストごとに一度だけ
    private $connection = null;

    // DB接続
    final public function getConnection() {
        // TODO: Implement getConnection() method.
        if ($this->connection === null) {
            if (self::$pdo == null) {
                self::$pdo= new PDO($GLOBALS['DB_DSN'], $GLOBALS['DB_USER'], $GLOBALS['DB_PASSWD']);
            }
            $this->connection = $this->createDefaultDBConnection(self::$pdo, $GLOBALS['DB_DBNAME']);
        }

        return $this->connection;
    }


    // DB のクリーンアップ
    public function getTearDownOperation() {
        return \PHPUnit\DbUnit\Operation\Factory::TRUNCATE();

    }

}

フィクスチャ用のデータセットを作ります。
今回は YAML ファイルを使います。

food:
  -
    id: 1
    name: karaage
    price: 450

それでは、テストコードを書きます。

<?php
require_once('Generic_Tests_DatabaseTestCase.php');
require_once('../../class/food.class.php');
use PHPUnit\DbUnit\DataSet\YamlDataSet;


class TestSample extends Generic_Tests_DatabaseTestCase {


    /**
     * フィクスチャ設定
     * @return \PHPUnit\DbUnit\DataSet\IDataSet|YamlDataSet
     */
    protected function getDataSet() {
        // TODO: Implement getDataSet() method.
        return new YamlDataSet(dirname(__FILE__).'\fixture_sample\sample.yml');
    }



    /**
    * テストコード
    */
    public function testGetDriverID() {


        // 初期状態の行数アサーション
        $this->assertEquals(1, $this->getConnection()->getRowCount('food'));

        // YAML ファイルで定義してあるデータから、アサーション用の比較データ取得。
        // 本来はフィクスチャ用のファイルと別に作るが、今回は Insert, Update, Delete を行わないため同じものを使用。
        $expectedTable = $this->getDataSet()->getTable('food')->getRow(0);


        // テストしたいメソッドの実行。
        $actual = Food::getById($expectedTable['id'], self::$pdo);


        // アサーション用のキー配列。created はテストしない。
        $check_keys_array = [
            'id', 'name', 'price'
        ];


        // 値が一致しているかのアサーション
        foreach ($check_keys_array as $key) {
            $this->assertEquals($expectedTable[$key], $actual[$key]);
        }

    }

}

このテストの実行はコマンドライン上で下記コマンドを打ってください。

phpunit -c phpunit.xml foodTest.php

以上でテスト完了です。
テストでテーブルに変更があっても変更したところを消去してくれるんで、かなーり便利だと感じました。

一応、注意しなければならないのが、PHPUnit としては DBUnit の開発を既に終えているそうです。
ですが、foak されたプロジェクト( https://github.com/kornrunner/dbunit )があるので、検討する価値はあるかなという感じです。

安全なプリペアードクエリ

PDO を使ってデータベース操作を行うときの、プリペアードクエリに少し躓いたのでまとめておこうと思います。

参考サイト:

PDO (PHP Data Object) とは

PHP の中で DB に接続し、操作を行うためのインターフェースです。DB の種類ごとに異なる関数を使う必要がなく、使用する DB の種類を変更する際にも対応しやすいという利点があります。

プリペアードクエリ とは

PDO が提供する、DB へ送る SQL 文を2段階に分けて実行する手法です。1段階目では、 SQL 文を 解析・コンパイル・最適化 し、2段階目で実行します。また、1段階目でパラメータをプレースホルダに置き換え、2段階目でそのプレースホルダにパラメータを渡すことが可能です。

プリペアードクエリを使うメリットは次の通りです。

  • SQL 文の解析~最適化は最初の1回だけ行えばよく、その SQL を何回も行うときに高速な動作が期待できる。
  • SQL 文の構成に入力値を使うときSQL インジェクションの危険があるが、適切にプレースホルダを用いることで容易にインジェクション対策ができる。

今回は、2つ目のインジェクション対策に焦点を当てます。

実際に使ってみる

今回はテキストボックスへの入力を受けて、下のテーブルから値を取り出し名前を表示するようにします。「きゅうり」を受け取ったら「きゅうり」を表示するという単純なものです。

+----------+-------+
| name     | price |
+----------+-------+
| キャベツ |   200 |
| きゅうり |    80 |
| かぼちゃ |   300 |
+----------+-------+

まず、プリペアードクエリを使わない方法でやってみます。コードは以下の通りです。

$sql = 'SELECT name FROM food WHERE name="'.$_POST['food'].'"';
    $foods = $db->query($sql)->fetchALL(PDO::FETCH_ASSOC);
    foreach ($foods as $food) {
        foreach ($food as $name) {
            print $name;
        }
    }

入力を行った結果を見てみます。

入力:「きゅうり」

入力:「きゅうり” or name=”キャベツ」

インジェクション対策を行ってないので、意図的に SQL 文を書き換えられています。

次に、プリペアードクエリを使いますが、入力値をプレースホルダで置き換えずにやってみます。コードは以下の通りです。

$sql = 'SELECT name FROM food WHERE name="'.$_POST['food'].'"';
    $stmt = $db->prepare($sql);
    $stmt->execute();
    $foods = $stmt->fetchALL(PDO::FETCH_ASSOC);
    foreach ($foods as $food) {
        foreach ($food as $name) {
            print $name;
        }
    }

結果を見てみます。

入力:「きゅうり」

入力:「きゅうり” or name=”キャベツ」

プリペアードクエリを使ったのにインジェクションされています。

それでは、プレースホルダを使ったプリペアードクエリを試してみましょう。コードは以下です。

$sql = 'SELECT name FROM food WHERE name=?';
    $stmt = $db->prepare($sql);
    $stmt->execute(array($_POST['food']));
    $foods = $stmt->fetchALL(PDO::FETCH_ASSOC);
    foreach ($foods as $food) {
        foreach ($food as $name) {
            print $name;
        }
    }

結果を見てみます。

入力:「きゅうり」

入力:「きゅうり” or name=”キャベツ」

ここだけ $foods を var_dump() してます。

インジェクション を防げました!

まとめ

今回は、

  • プリペアードクエリを使わない通常クエリ
  • プレースホルダを用いないプリペアードクエリ
  • プレースホルダを用いたプリペアードクエリ

の3通りについて試し、プレースホルダを用いたプリペアードクエリのみがインジェクションを防ぐことができました。prepare() でクエリを解析した後にプレースホルダに値を渡すので、不適切な入力を防いでくれています。

なので、入力値を用いて DB を操作する際には、プレースホルダを用いてプリペアードクエリを使うのがほぼ必須になります。構文によってどうしても難しい場合は、かなり厳格な入力バリデーションが必要になります。

GitHubで誤ってプッシュしたコミットを消去する

初めまして!今年の春から新卒でオンラインコンサルタントに入社した者です。

先日、DB アクセスのためにパスワードをソースコードに書いたまま GitHub の公開リポジトリにプッシュすると言う失態を犯しましたので、その時に行った対処と陥った事態をここに記しておきます。

(1)捨てパスワードを用意し、コミット&プッシュ

大事なパスワードが GitHub で公開され続けるのはまずいので、パスワードを見られても構わない捨てパスワードに変更し、コミット&プッシュします。しかし、これでは何も安心ではありません。

GitHub にて該当ファイルのページに行き、「History」ボタンを押してみましょう。すると、ファイルのコミット履歴が表示されます。その中から、パスワードを変更したコミットを選択してください。

はい。しっかりと残っています。これも消さなくては。

(2)ローカルリポジトリでコミット履歴を消去し、プッシュする。

初めに言わなければならないことがあります。これ以降については、作業を並行して行いながら読まないでください。最後に、落とし穴について述べます。

まず、ターミナルで以下のコマンドでコミット履歴を確認します。

$ git log
commit 94ae37722f47d8b76b6e14f5288b37b368b7cc05 (HEAD -> master, origin/master, origin/HEAD)
Author: 森近 楓 <kaede.morichika@onlineconsultant.jp>
Date:   Wed Apr 22 13:45:17 2020 +0900

    安全なパスワードに変更。

commit 9e6642fb642983e8c3a9a8b69c7a4716cf62ed36
Author: 森近 楓 <kaede.morichika@onlineconsultant.jp>
Date:   Wed Apr 22 13:44:17 2020 +0900

    大事。

commit 0a98ca4c6bd3244d06062d23901157c5598c0dfd
Author: 森近 楓 <kaede.morichika@onlineconsultant.jp>
Date:   Wed Apr 22 13:41:41 2020 +0900

    DB接続設定

コミットが最新のものから順に表示されます。今回は、1つ目と2つ目を消したい。以下のコマンドを使います。

$ git reset --hard HEAR~2 # 消すコミットの数
HEAD is now at 0a98ca4 DB接続設定

git log します。

$ git log
commit 0a98ca4c6bd3244d06062d23901157c5598c0dfd (HEAD -> master)
Author: 森近 楓 <kaede.morichika@onlineconsultant.jp>
Date:   Wed Apr 22 13:41:41 2020 +0900

    DB接続設定

ローカル環境からコミットの内容が消えました。さて、リモートにプッシュしましょう。以下のコマンドを使います。

$ git push -f

-f は、強制的に行うためのオプションです。-f なしでは、ブランチの状態が最新じゃないと判定されエラーで弾かれます。GitHub を確認します。

リモートにおいてもコミットが消えています。

(3)落とし穴

これで、公開リポジトリから大事な情報がなくなり一安心としたいところですが、実は消したコミットに該当する編集部分も消えています。もし、消したい部分(今回で言えば大事なパスワード)以外もコミット内容に含まれているのなら、コミット内容の魚拓をテキストファイルなどに作ってから行うことをお勧めします。

また、今回は最新の2つのコミットを消したかったので git reset で良かったのですが、遥か前のコミットを消そうとすると、git filter-branch というコマンドを使う必要があります。

(4)おわりに

GitHub に誤って大事な情報をプッシュしてしまった時の対処法について紹介しました。初心者エンジニアの皆さんは、コミット&プッシュの前にはしっかりと内容を確認し、僕みたいなことはしないようにしましょう。

参考サイト:https://qiita.com/dtan4/items/34e41e3bd40a43fd8cbf