営業の枠から飛び出せ!
営業の枠から飛び出せ!
2019.06.18

【これからエンジニアになる人必見】やってはいけないコーディング3選

やなさん

みなさんこんにちは。2019年4月にLIGに入社したバックエンドエンジニアのやなさんです。

現代のエンジニアたるもの、デザインの基礎知識も必要と思い、齢35にして愛する大阪を離れ、LIGが運営する「デジタルハリウッドSTUDIO by LIG」に通うため、満を持して遅めの東京進出を果たしました。

そして、4月からLIGで働くことになりました。
卒業制作はこちら ※現在も鋭意作成中のため、開けないページ多数です。

この度4月でエンジニア歴15年目に突入したということもあり、まずはエンジニアらしいことを書いてみたいと思います。

15年もエンジニアをしていると、たくさんのソースコードを見る機会がありました。経験としては規模が大きめの案件が多かったです。またプロジェクト規模に比例して、ソースコードもボリューム多めでした。なので、同じ15年目のエンジニアの人よりはたくさんの実務レベルのソースコードを見てきたと自負しております。

そんな多くのソースコードを見てきた私が、「うそやんっ!!」って声を出してしまうほど悲しくなってしまったプログラムを厳選してご紹介したいと思います。題して「やってはいけないコーディング3選」です。

研修期間を終え、これからエンジニアを目指す、もしくはこれから本格的に開発をする方、くれぐれも真似しないようお願いしますね……!

やってはいけないコーディング3選

何で囲んだ? 「try catch」

これは、フレームワークを使用経験が少なそうな人のプログラムでよく見たパターンです。

どんなものかというと、まずは以下のサンプルをご覧ください。

namespace App\Http\Controllers;

use Illuminate\Http\Request;
use App\Base\Controller\BaseController;
use App\Models\User\UserEloquent;
use Exception;

class HogeController extends BaseController
{
    public function post(Request $request)
    {

        try {
            $model = New UserEloquent();
            $model->email = $request->email;
            $model->name = $request->name;
            $model->save();
            //
            $this->view = view('pages.home.home');
        } catch (Exception $e){
            // 例外が発生した場合の記述
            $message = $e->getMessage();
        }

        return $this->renderer();
    }
}
try catchとは
try内で例外(プログラムとして予期しない動作)が発生した場合にcatch内に適切に処理を記述する箇所です(try catchをしていないとプログラムはそこで異常終了してしまいます)。

なにがだめなの?

例外が発生するのを防ぐためにtry catchを記述しているなら問題ないのでは?? と思った方、要注意です。

ですが、サンプルプログラムではエラーのメッセージを$messageに格納しているものの、その値を返却しないまま処理が終了しています。catch内で適切な例外処理をしないということは、例外がなかったこと(処理が正常終了)になってしまいます。

上記のような記述があることで、本来はプログラムとしては正しく処理されていないにもかかわらず、システム的には正しく処理されたと判断されてしまいます。

いままであった私の最悪なパターンでは、画面に「登録が完了しました」と出ているのに、プログラムを追いかけた結果データが登録されていなかったことがあり、涙を流した記憶があります……。

じゃあ、どうすればいいの?

相当な理由がない限り、個々のプログラムではtry catchすべきではないと私は思ってます。

いまのフレームワークはたいがい、例外(Exception)が発生した場合に必ず呼び出されるプログラムがあり、そこでフレームワークが最低限の例外処理を適切な処理をしてくれるクラスが準備されています。

その例外処理クラスを継承し、システムに応じた適切な例外処理を記述しておく作りにしておきましょう。

システムテストを実施する担当者は、プログラムを1行単位に把握しているわけではありません。エラーが出るからといって勝手にcatchしていると、システムのリリース稼働後に大変な目に遭うことでしょう。

その数字はなにを指している? 「マジックナンバー」

これも保守ではかなり辛いプログラムです。

まずはサンプルから。

if ($hoge == 1) {
	// hogeの値が1だった場合の処理を記述
} else {
	// hogeの値が1以外だった場合の処理を記述
}

これ、なにがしたいかわからないですよね。僕もわからないです。
わかることは$hogeの値が1だったらif文の中のプログラムが実行され、1じゃなければ実行されない。僕にはそれだけしかわかりません。

なにがだめなの?

プログラムのソースコードに具体的な数値が記述されているものをマジックナンバーと呼びます。

マジックナンバーとは
何らかの識別子もしくは定数として用いられる、プログラムのソースコード中に書かれた具体的な数値である。そのプログラムを書いた時点では製作者は数値の意図を把握しているが、他のプログラマーまたは製作者本人がマジックナンバーの意図を忘れたときに閲覧すると「この数字の意味はわからないが、とにかくプログラムは正しく動く。まるで魔法の数字だ」という皮肉を含む。

保守対応では、プログラム構造はおろか、システム自体が理解できていない状態で対応を求められることもしばしばあります。

そんななかでプログラムにマジックナンバーがあると、値がなにを指しているのかわからず、トラブル報告として「xxxxのときの処理がおかしい」と言われた場合に、どちらがxxxxのときか判別することができず、トラブルで急いで解決しないといけないのに、1の意味を探す旅(調査)に出かけなければなりません。

じゃあ、どうすればいいの?

こういった値は定数として定義することが重要です。また定数名もほかの人が見ても何の値を指しているかわかる定数名にしておくことです。定数名に意味があると、保守を担当するエンジニアはプログラムが理解しやすくなるため、より迅速な対応を行うことができるでしょう。

急いで開発することで、こういったプログラムが発生しがちですが、最初に作る人の手間暇(優しさ)がシステムに関わる人たちをハッピーにすることでしょう。

読みづらくね? 「延々続くコントローラークラス」

私にとっては一番イヤなプログラムです。

まずはサンプルから。

namespace App\Base\Controller;
class HogeController extends Controller
{
    public function __construct()
    {
        // コンストラクタ
    }

    public function post()
    {

        if ($hoge == CODE_A) {
            // hogeがCODE_Aの場合




            // 何百行もの処理を記述







        } else if ($hoge == CODE_B) {
            // hogeがCODE_Bの場合




            // 何百行もの処理を記述







        }

    }

}

黒い部分多いと思いましたよね。

上記サンプルでは処理を記述しておりませんが、MVCの呼び出されたコントローラの関数内にすべての処理を記述するようなプログラムがたまにあるんです。私はこういうプログラムを発見するとできるかぎり見てみぬフリをして、そっと×ボタンを押しがちです。

なにがだめなの?

こちらはそもそもMVCのデザインパターンに沿っていないということもありますが、正直そんなことはどうでもいいんです。なにが嫌かというと、「結局ここの処理でなにをすべきか?」がぱっと見てわからないことです。

たとえば曖昧なトラブル報告があった場合、エンジニアはかなりの確率で「仮設を立てて調査する」ことがほとんどだと思います。仮にその仮設が間違えていたとしても、また新たに仮設を立てて違うプログラムを調査する。その繰り返しで原因を特定していきます。そのときに知りたいことは、プログラム1行のコーディングではなく、調査対象のプログラムでざっくりと「なにしてるか?」が一番知りたいんです。

最終的には全体的にプログラムを追う必要があるのですが、こういった処理があると、すべての行を読み解かないと次に進めないため、一つひとつのプログラムの調査時間が膨大になってしまいます。

じゃあどうすればいいの?

基本的には一番に呼び出されるコントローラは、modelとviewをつなぐ要素なので、処理自体は何かの関数を呼び出すだけの処理になってるのが望ましいと考えます。イメージとしては、コントローラ内の呼び出されるプログラム1行1行が「目次」的な要素になっているイメージです。そうすることで呼び出された処理でなにをしているかがぱっと見てわかるため、「仮設を立てて調査する」スピードが格段に上がります。

保守のトラブル対応は即時性が求められます。プログラムは作って終わりではなく、そこからがはじまりなので、次にこのソースを見るエンジニアの人を思って、見やすいプログラムを作ることを心がけましょう。

まとめ

ちなみに私自身も、上記3つのコーディングをしてしまった経験があります。特に2つ目と3つ目はどうしても急いでいると、よくやりがちなパターンです。これからエンジニアになっていくみなさんは、こんなプログラムを作らないように、たくさんの書籍やプログラムを見て、日々エンジニアとしての自身が書いたコーディング位説明できるスキルを磨いていってください。

プログラムは作って終わりではありません。むしろ作ってから運用することが重要です。なので、自分だけがわかるようなプログラムではなく、一定のエンジニアが見れば理解しやすいと思うプログラムを作成するよう心がけていきましょう!

システム開発は計画的に。

それではまた、やなさんでした。

やなさんが通った
LIGのWebデザインスクールはこちら!