give IT a try

プログラミング、リモートワーク、田舎暮らし、音楽、etc.

ひどいコードをメンテしてきたからこそ実感する、良いコードや良い設計の大切さ

はじめに

先日、社内で「良いコードの書き方やお作法、プログラミングの原則って、どうやったら身に付くんだろうねえ?」という話になりました。
もちろん、「本を読んで勉強する」っていのも勉強法のひとつなんですが、そもそも、もっと強烈なモチベーションがないと、必死になって良いコードの書き方やプログラミングの原則って勉強できないのでは?なんて思ったりします。

強烈なモチベーションというのは、たとえば、

いったい何なん!?このスパゲティコードは!!!
なんでこんなコードを俺がメンテしなきゃあかんの!!??
あ~、もう最悪や!!俺はこんなコード、絶対に書かへんぞ!!!!

っていうぐらいのモチベーションです。
というか、これは単純に僕のケースですね、はい。

幸い、ソニックガーデンに入ってからは、周りのプログラマがみんなちゃんとしているので、そんな思いをすることはほぼなくなりましたが、前職、前々職ではそんなことがよくありました・・・というよりむしろ、ほとんどそんな経験ばかりだったかもしれません。
昔のブログにはそのへんの愚痴をつらつらと書いてあったりするものもあります(苦笑)。

というわけで、このエントリでは過去に遭遇した「アンチパターン」とその解決策をまとめてみようと思います。

主要な変数が全部連番、かつあらゆる変数がグローバルなシステム

かれこれ十数年前、僕がこの業界に入ってきて最初に担当したのはVB(Visual Basic)5やVB6の保守案件でした。
当時はすでに.NETフレームワークが登場し始めたぐらいの時期だったので、VB自体がやや時代遅れだったのですが、僕が担当していた案件はCOBOL時代のソースコードをほぼそのままVBに移植したアプリケーションでした。

画面に表示される項目はすべて INT001、STR002 のような「型名ハンガリアン+連番」になっており、なおかつそれがすべてグローバル変数として宣言されていました。
実際はINT001に商品コード、STR002に商品名、みたいな割り振りになっているのですが、コード上の表記はあくまでINT001、STR002です。

' モジュール名も当然連番
Module T01S00031
  Dim INT001 As Integer
  Dim STR002 As String
  Dim STR003 As String
  ' 以下変数が何十個も並ぶ
End Module

さらに最悪なのは「i, j, k」のようなループ用の変数もグローバル変数で宣言されていたことです。
うっかり使い方を間違えると、ループ開始時の初期値がおかしくなったりします。

Module T01S00031
  Dim INT001 As Integer
  ' (以下省略)
  ' モジュールのトップレベルでループ用の変数を宣言
  Dim i, j, k As Integer

  Sub HOGE_RTN()
    For i = 0 To 5
      ' グローバル変数iでループ処理・・・
    Next i
  End Sub
End Module
テーブル名やカラム名も全部連番

コードの上の変数だけでなく、テーブル名やカラム名も全部連番です。
M01S0001は商品マスタ、C0100001は商品コード、みたいなそんな世界です。

ずっとその案件を保守している先輩プログラマは「M01S0001は商品マスタだから、ここは~」みたいに、脳内で変換テーブルを組めていましたが、新人だった僕はそんな芸当ができるはずもなく、いつもテーブル定義表を片手に「ええと、M01S0001は・・・商品マスタ、C0100001は・・・・商品コードか。で、C0100002は・・・??」みたいに調べていました。

こんな状況なので、コードを書くのに時間がかかるのは言うまでもありません。

解決策

CODE COMPLETE、リーダブルコードを読みましょう!!
「人間にとって理解しやすく、保守しやすいコードとは何か」を教えてくれます。

CODE COMPLETE 第2版 上 完全なプログラミングを目指して

CODE COMPLETE 第2版 上 完全なプログラミングを目指して

リーダブルコード ―より良いコードを書くためのシンプルで実践的なテクニック (Theory in practice)

リーダブルコード ―より良いコードを書くためのシンプルで実践的なテクニック (Theory in practice)

  • 作者: Dustin Boswell,Trevor Foucher,須藤功平,角征典
  • 出版社/メーカー: オライリージャパン
  • 発売日: 2012/06/23
  • メディア: 単行本(ソフトカバー)
  • 購入: 68人 クリック: 1,802回
  • この商品を含むブログ (140件) を見る

余談

その案件がCOBOLからの移植と知ったのは、ずいぶんあとになってからのことでした。
案件に入った頃は新人だったこともあり、「これがプログラムってやつなんだね~」と思いながら、何もわからずそのコードを(四苦八苦しつつ)保守していました。

まあ、COBOL時代なら「コンピュータの都合に人間が合わせる」というのも理解できますが、そういった制限がなくなったのであれば、人間が読み書きしやすいコードを書きたいものです。

カンマ区切りで複数のコード値を格納しているテーブル

売上げテーブルの商品コードカラムに、

ABC001, LMN123, XYZ111

のように、カンマ区切りの文字列が格納されているプログラムを見たことがあります。
画面上にはこれが

肉, にんじん, たまねぎ

のようになって、商品名が出てくるのですが、これを実現するためにストアドファンクションの中でいちいち文字列をカンマでぶったぎって、ループを回して商品マスタから1件ずつ商品名を取得していました。

適当な擬似コードで表現するとこんな感じです。

FUNCTION F_ITEM_NAME(CODE_TEXT)
  CODES = SPLIT(CODE_TEXT, ',');
  RET = '';
  NAME = '';
  FOR CODE IN CODES
    SELECT NAME INTO NAME FROM SHOHIN WHERE CODE = CODE;
    RET += NAME + ',';
  END
  RETURN RET;
END

売上げ一覧を表示する場合はこのストアドファンクションを何十回も実行することになるので、それはそれは遅~~~~いプログラムになっていました。

SELECT
  CODE,
  NAME,
  -- 100件データがあれば、100回ファンクションが呼ばれる
  F_ITEM_NAME(ITEM_CODE) AS ITEM_NAME,
  -- しかも他にも同じようなファンクションが呼ばれてたりする・・・
  F_ITEM_KUBUN_NAME(KUBUN) AS ITEM_KUBUN,
  F_ITEM_SIZE(SIZE_CODE) AS ITEM_SIZE
FROM
  ORDERS
WHERE
  ORDER_DT > '2016-07-01'
解決策

SQLアンチパターンを読みましょう!(ジェイウォークというアンチパターンで載っています)
そしてテーブルの正規化についてしっかり学びましょう!!

SQLアンチパターン

SQLアンチパターン

あわせて読みたい

なお、この件に関する詳しい話は以下のエントリに載っています。

Ruby on Railsでもserializeを乱用すると同じような状況が発生するので注意してください。

エラーを握りつぶして無視するシステム

エラーが発生したら「うるさい、黙れ!」と握りつぶして、何事もなかったように処理を再開するシステムも見たことがあります。
たとえばこんな感じです。

public int getDataTable(string id) 
{
  try {
    DataTable dataTable = new DataTable(); 
    // (データベースにアクセスするコードが入る)
    //処理結果は戻り値じゃなくてインスタンス変数に格納!?
    this.table = dataTable;
    return 1; 
  }
  catch (Exception e) {
    //エラーが起きたら全部握りつぶすよ!
    //でも一応メッセージだけインスタンス変数に保存しておくね♪
    this.errMsg = e.Message; 
    return 0; 
  }
}

//戻り値が1であろうと0であろうと100であろうと、確認されることなく処理が進む(涙)
int val = someObj.getDataTable(id);

//エラーが起きたので、tableはnullになる・・・
DataTable table = someObj.table; 

//tableがnullなので当然例外発生!
//しかし、元の例外の情報はどこにも保存されておらず、真相は闇の中・・・
string name = table.Rows[0][0].ToString(); 

「なんか動きがおかしい」
「なんかデータの整合性がとれてない」
「ユーザーから問い合わせが来てるけど、ログを見ても特にエラーが出てない」

・・・みたいな現象が起きるのでコードをじっくり追っかけていくと、エラーが握りつぶされてた!!ということが何度かありました。

解決策

エラー処理のベストプラクティスを学習しましょう!
僕は.NET時代に以下のWeb記事を読んで勉強しました。

https://blogs.msdn.microsoft.com/nakama/2008/12/29/net-part-1/

この考え方はRailsでもほぼ応用が利くので、Railsでの実践例を以下の記事にまとめています。

あわせて読みたい

ちなみに先ほどのコードは例外を握りつぶしている点のみならず、「戻り値で処理の成功や失敗を表そうとしている点」や、「処理の途中で必要になるデータを、グローバル変数であるかのごとくインスタンス変数に格納している点」もよろしくないです。
「処理の途中で必要になるデータを、グローバル変数であるかのごとくインスタンス変数に格納するな!!」という話は以下のQiita記事にまとめています。

(悪い話ではなく、良い話)最新のフレームワークやオブジェクト指向プログラミングを活用したシステム

ここまではひどいコードの話ばかりしてきましたが、ひどいコードをたくさん見てきてるからこそ、良いコードや良いフレームワークの素晴らしさを知ったときの感動がいっそう際立ちます。

僕が最初に担当したのはCOBOLチックなVBプログラムだと書きましたが、入社して1年ぐらいした後に参加したJava案件が僕にとって素晴らしいものでした。
この案件は当時最新だったStruts(Webフレームワーク)やHibernate(O/Rマッピングツール)といったフレームワークを使った案件でした。

STRUTS・イン・アクション

STRUTS・イン・アクション

HIBERNATE イン アクション

HIBERNATE イン アクション

  • 作者: Christain Bauer,Gavin Ki,倉橋央,勝嶌和彦
  • 出版社/メーカー: ソフトバンク クリエイティブ
  • 発売日: 2005/12/28
  • メディア: 大型本
  • 購入: 3人 クリック: 109回
  • この商品を含むブログ (35件) を見る

「1箇所変更すれば、全画面が変わる」の衝撃

この案件はそれまでのVBプログラムに比べると、作りやすさに天と地ほどの差がありました。
それまでは「プログラムはコピペして作れ」と言われていて(ひどい)、ちょっとした仕様変更する場合でもコピペした部分を全部探して、一個ずつ変更、変更、変更・・・を繰り返していました。

しかし、このJava案件ではオブジェクト指向を活用したプログラムの再利用性や、重複を許さないDRYの考え方が導入されていて、コードを1箇所変えるだけですべての画面にその変更点が適用されました。
これは当時の僕にとって本当に衝撃的でした。

また、O/Rマッピングツールを使うと簡単にデータベースとデータの出し入れができるのも非常に便利でした。

「今ドキのアプリケーション開発」を知らなかった僕は、旧石器時代から急に現代にタイムスリップした原始人のような状態でした。

オブジェクト指向プログラミングが便利な例(テンプレートメソッドパターン)

当時のJava案件で「オブジェクト指向ってすごく便利!!」と思った例のひとつが、デザインパターンの「テンプレートメソッドパターン」を活用した仕組みです。
テンプレートメソッドパターンを使うと、「SQLを実行して、その結果をCSVに出力する」という基本ロジックは再利用しつつ、「実行するSQL」や「出力データの加工」など、出力対象によって異なる処理をサブクラスで定義することができます。

残念ながらJavaは何年も書いていないので当時のコードをそのまま再現することはできません。
ここでは僕が今一番得意なRubyのコードで同じようなコードを書いてみます。

# CSV出力の共通処理を定義した親クラス(抽象クラス)
class CsvGenerator
  # CSV出力の共通処理
  def generate_csv(file_name)
    output_path = Rails.root.join('tmp', file_name)
    File.open(output_path, "w") do |file|
      # sqlはサブクラスのsqlメソッドから取得する
      records = ActiveRecord::Base.connection.execute(sql)
      records.each do |row|
        # 必要に応じて出力データを加工する
        cols = process_row(row)
        file.puts(cols.join(','))
      end
    end
  end

  # デフォルトは単純にSQLの実行結果をそのまま出力する
  def process_row(row)
    row.values
  end
end

# ユーザーCSVを出力するサブクラス
class UserCsvGenerator < CsvGenerator
  # ユーザーCSV用のSQL
  def sql
    'SELECT id, name, email FROM users ORDER BY id'
  end

  # 出力データを加工する
  def process_row(row)
    cols = []
    cols << row['id']
    # 名前は"Junichi Ito (foo@example.com)"のように出力する
    cols << "#{row['name']}(#{row['email']})"
    cols
  end
end

# ブログポストCSVを出力するサブクラス
class PostCsvGenerator < CsvGenerator
  # ブログポストCSV用のSQL
  def sql
    'SELECT id, title, category FROM posts ORDER BY created_at'
  end

  # 出力データは加工しない(デフォルトの実装を使う)
end

# ユーザーCSVを出力する
generator = UserCsvGenerator.new
generator.generate_csv('users.csv')

# ブログポストCSVを出力する
generator = PostCsvGenerator.new
generator.generate_csv('posts.csv')

上のコードはRubyですが、当時はこんなコードをJavaで書いていたと想像してください。
こんなふうにすると、共通処理は共通処理で再利用しつつ、状況によって異なる処理だけを柔軟に実装することができます。

僕の中に生まれた意識の変化

この案件で学んだことは次のようなことです。

  • 最新の技術を使えば開発がすごくラクになる。特にオブジェクト指向プログラミングがこんなに便利なものだとは知らなかった。
  • 良い設計と良いコードがあれば開発がすごく楽しい。逆に悪い設計、悪いコードだと開発が苦痛で仕方が無い。
  • 最新の技術を学ぶには海外から情報を仕入れなければいけない。英語力はゆえに必須になる。
  • 先輩プログラマは常に正しいとは限らない。間違っている知識を教えてくる場合もある。(「コピペして作れ」とか!!)

これ以降、僕は「アホみたいなコードを書いたりメンテしたりしたくない!最新の技術を使って、ラクに楽しく開発したい!!」と考えるようになり、主体的に最新の技術や良いコードの書き方を学ぶようになりました。

あわせて読みたい

旧石器時代から現代にタイムスリップしたときの詳しい話はこちらにまとめています。

その他のエピソードあれこれ

この手の話はしゃべり出すとキリがないので、ここからは昔書いたブログのエントリを貼っていきます。
興味を持った内容があればチェックしてみてください。

本当に役に立った技術書あれこれ

良いコードや良い設計を学ぶのに役立った本をいろいろとまとめたエントリです。

昔の常識、今の非常識!?

昔は「そうせざるを得なかった」場合でも、最新の言語ではその常識が当てはまらないこともよくあります。

テストも自動化したい!

テストを毎回手作業と目視で確認する経験がある人ほど、自動化テストのありがたみを感じやすいかもしれません。

データベース設計がイケてないとプログラムも悲惨

ダメダメなデータベースで開発したり運用したりするのは本当に大変です。

ダメなコードをツールであぶり出す

前職ではダメなコードをツールを使って定量的にあぶり出す、ということもやっていました。

まとめ

というわけで、このエントリでは僕がこれまでに見てきた「ひどいプログラム」とその解決策(参考文献)、それと主体的に最新の技術や良いコードの書き方を学ぶようになったきっかけをまとめてみました。

冒頭にも書きましたが、こんな感じで僕は

いったい何なん!?このスパゲティコードは!!!
なんでこんなコードを俺がメンテしなきゃあかんの!!??
あ~、もう最悪や!!俺はこんなコード、絶対に書かへんぞ!!!!

・・・という経験を幾度となく繰り返してきました(苦笑)。

もしかして、最初から恵まれた環境でコードを書いていると、良いコード、良い設計、良いフレームワークのありがたみが感じにくくて、あまり勉強に身が入らないかも?なんて思ったりもするんですがどうなんでしょう??

いや、これはあくまで僕の個人的な体験談なので、みなさんにはそれぞれ違ったエピソードがあったりするのかもしれません。
みなさんもご自身のブログ等で「良いコードの書き方やプログラミングの原則を学ぶ方法」を説明されてみてはいかがでしょうか?
いろんなエピソードが聞けることを楽しみにしています!