プログラミングの際にしてはいけないこと

最近、ダメなコードに出会ったので、反面教師として参考までに書いておきます。

• グローバル変数を使う
• SQLインジェクション対策がされていない
• クラス内不要なメソッドが存在する

簡単に説明しておきます。こういうソースを見るとツッコミどころを沢山探したくなってしまいますね。

グローバル変数を使う

グローバル変数は、プログラミングをする際に楽になるかもしれませんが、意図しない動作(途中でグローバル変数の値自体が変わるなど)を引き起こす可能性が高く、どうしても仕方ない場合を除いて使うべきではありません。そして大抵の場合は使う必要はないです。

SQLインジェクション対策がされていない

SQL インジェクション自体は、ウェブで調べればいたるところで説明が書かれてあるので、書きませんが、いくら信用できるところから得られた値であっても、 Validationを行うべきですし、prepared statementを使ってSQL文を実行すべきだと思います。*1

とりあえず簡単にダメなコード例をPHPを使って書いておきます。

class User

{

	function getTrustID () {

		// サーバ上からIDを取得

	}
	function getTrustPassword () {

		// サーバ上からパスワードを取得

	}
	function isUser () {

		$id       = $this->getTrustID();

		$password = $this->getTrustPassword();

		$sql = "SELECT count(*) FR0M users WHERE id = '$id' AND password = '$password'";

		// 以下、SQL文の実行へと続く

	}

}

サーバ上に格納するロジックがどこかにあるのですが、そこにバグなどで、passwordに「';DELETE FROM users;」なんて文字列が入っていたらどうなるのでしょう？

クラス内不要なメソッドが存在する

こんな書き方は止めましょう。Db::execute()でもDb::exec()でも同様処理を実行できるので、利便性を高めたいのかもしれませんが、こういうすべきじゃないですよね。

class Db

{

	function execute () {

		$this->exec();

	}
	function exec () {

		// 処理の実行

	}

}

*1:プログラム言語自体がprepated statementをサポートしている場合ですが