この記事はTLB Enjoy Developers Advent Calendar 2022 7日目の記事です。
はじめに
こんにちは！ゆっきーです！！普段はこのカレンダーを主催している会社でサーバーサイドエンジニアをやっています！
さて、突然ですが、皆さんはソースコードを書くとき、どんなことを意識して書いていますか？
- 楽しく思うがまま
- わかりやすい命名を心がける
- 効率的な処理を心がけたい
などなど、色々だと思います。
自分はといますと、今でこそ会社や個人活動などで色々学び大きく意識が変わったものの、入社前はわかりやすい命名を用いておしゃれでかっこいいコードを書くことに憧れていました
また、ファイルやフォルダの分割も断片的な知識で、よく考えずにそれっぽくやってました。。
さて、↑こんな意識で書いたコードが果たしてどんなものなのか。一部の人には想像がつくかもしれませんが、そこには大量のスパゲティコードが、、
本文章では、入社して半年、そこそこ勉強・経験した新卒エンジニアの自分が
- 仕事でコードを書くことの難しさ
を振り返りながら、日頃感じているコードを書くことの難しさについて整理していきます！
お手柔らかにお願いします🔰
対象読者
- 就業経験の少ない学生エンジニア
- 他の新卒が何を考えているのか気になる新卒エンジニア
- 新人が何を考えているのか気になるエンジニアの皆さん
コードは設計書である
コードは設計書である
これは、かの有名な「プリンシプル・オブ・プログラミング」の最序盤に登場するプリンシプルの一つです。ざっくりとした内容としては
「設計によるアプトプットはコードでしかありえず、数あるドキュメントの中で真にエンジニアリングのドキュメントと言えるものはコードのみである」
といった内容になってます。
では、ドキュメントって何のためにあるのでしょうか？多くの場合対象物そのものの内容について読み返すためだと思います。
つまり、コードは読んだ時にその内容が自分を含めた誰にでも伝わるように書かなければならないわけです。
この行為は当然難しいと思うのですが、それ以前の問題として、その難しさを認識すること自体が難しいことであるように自分は感じています。
以下、いくつか例をあげてみます。
伝わらないコード
伝わらないコードはいつだって意識の欠如から爆誕します。コードとは自然に任せていれば無秩序に向かうものなのですから、仕方がないですね。
ここで、学生時代に自分がノリと勢いで書いたFlaskのコードの一部を見てみましょう。
@app.route('/', methods=['POST','GET'])
def index():
if request.method == 'POST':
upfile = request.files.get('upfile')
kind = request.form.get('kind')
if upfile is None:
flash('No File Part')
return redirect('/')
if upfile.filename == '':
flash('No selected file')
return redirect('/')
if kind is None:
flash('No kinds of music')
return redirect('/')
if allowed_file(upfile.filename):
filename = secure_filename(upfile.filename)
upfile.save(os.path.join(app.config['UPLOAD_FOLDER'], filename))
id = 'IM_' + uuid.uuid4().hex
score_maker.EdgeDetection(filename,id + '_edge.png')
if kind == 'ryukyu':
score_maker.OptToRyukyu(id + '_edge.png',id + '_ryukyu.png')
scoreName = id + '_ryukyu.'
score_maker.ImageToScore(scoreName + 'png',scoreName + 'mid')
score_maker.MakeImages(scoreName + 'png',scoreName)
midi_to_mp3.midi_to_mp3(scoreName + 'mid',scoreName + 'wav')
#cols = score_maker.IMGCols(scoreName + 'png')
#score_maker.Mp4Maker(scoreName,scoreName + 'mp4',cols)
#clip = mp.VideoFileClip(os.path.join(app.config['UPLOAD_FOLDER'], scoreName + 'mp4')).subclip()
#video = clip.set_audio(mp.AudioFileClip('./files/sounds/' + scoreName + 'mp3'))
#video.write_videofile(filename = os.path.join(app.config['UPLOAD_FOLDER'], 'second_' + scoreName + 'mp4'), codec = 'mpeg4', audio = './files/sounds/' + scoreName + 'mp3', audio_codec = 'libmp3lame')
#audioFile = mp.AudioFileClip('./files/sounds/' + scoreName + 'mp3')
#clip.write_videofile(filename = os.path.join(app.config['UPLOAD_FOLDER'], 'second_' + scoreName + 'mp4'), codec = 'mpeg4', audio = audioFile, audio_codec = 'libmp3lame')
#clip.close()
#video.close()
return render_template('view.html',image='static/files/images/'+scoreName+'png',sound='static/files/sounds/'+scoreName+'wav')
return render_template('index.html')
これは「ルートに対してGETリクエストが送られてきたらindex.htmlを、POSTで画像とその種類が送られてきたらそのテーマに応じて自動生成した楽曲と画像を埋め込んだview.htmlを返す」といったコードなのですが、わかりましたか、、？(そもそも実装が不十分だったり、動作が不安な点は今回は不問としましょう、、。)
じっくり読めば内容はわかりますが、少なくともドキュメントにはなっていませんね。。混沌としています
では、これをこんな感じに変えてみたらどうでしょうか？
@app.route('/', methods=['POST','GET'])
def index():
+ """ルートへのリクエストのハンドラー
+
+ GETリクエストの場合はindex.htmlを返す。POSTリクエストの場合はRequestBodyから画像と曲のテーマを受け取り、曲を生成したのち、view.htmlを返す
+
+ Returns:
+ Getリクエストの場合はindex.htmlが返される。
+ POSTリクエストの場合はview.htmlが返される。
+ """
if request.method == 'POST':
- upfile = request.files.get('upfile')
- kind = request.form.get('kind')
+ music_theme = request.form.get('kind')
+ upload_image = Image.of(request.files.get('upfile'))
- if upfile is None:
- flash('No File Part')
- return redirect('/')
-
- if upfile.filename == '':
- flash('No selected file')
- return redirect('/')
-
- if kind is None:
- flash('No kinds of music')
- return redirect('/')
-
- if allowed_file(upfile.filename):
- filename = secure_filename(upfile.filename)
- upfile.save(os.path.join(app.config['UPLOAD_FOLDER'], filename))
- id = 'IM_' + uuid.uuid4().hex
- score_maker.EdgeDetection(filename,id + '_edge.png')
- if kind == 'ryukyu':
- score_maker.OptToRyukyu(id + '_edge.png',id + '_ryukyu.png')
- scoreName = id + '_ryukyu.'
-
- score_maker.ImageToScore(scoreName + 'png',scoreName + 'mid')
- score_maker.MakeImages(scoreName + 'png',scoreName)
- midi_to_mp3.midi_to_mp3(scoreName + 'mid',scoreName + 'wav')
+ midi_music = MidiMusic.of(upload_image, music_theme)
+ mp3_music = MusicConverter.convert_to_mp3_from(midi_music)
- #cols = score_maker.IMGCols(scoreName + 'png')
- #score_maker.Mp4Maker(scoreName,scoreName + 'mp4',cols)
- #clip = mp.VideoFileClip(os.path.join(app.config['UPLOAD_FOLDER'], scoreName + 'mp4')).subclip()
- #video = clip.set_audio(mp.AudioFileClip('./files/sounds/' + scoreName + 'mp3'))
- #video.write_videofile(filename = os.path.join(app.config['UPLOAD_FOLDER'], 'second_' + scoreName + 'mp4'), codec = 'mpeg4', audio = './files/sounds/' + scoreName + 'mp3', audio_codec = 'libmp3lame')
- #audioFile = mp.AudioFileClip('./files/sounds/' + scoreName + 'mp3')
- #clip.write_videofile(filename = os.path.join(app.config['UPLOAD_FOLDER'], 'second_' + scoreName - 'mp4'), codec = 'mpeg4', audio = audioFile, audio_codec = 'libmp3lame')
- #clip.close()
- #video.close()
- return render_template('view.html',image='static/files/images/'+scoreName+'png',sound='static/files/sounds/'+scoreName+'wav')
+ return render_template('view.html',image=upload_image.get_path(),sound=mp3_music.get_path())
+ if request.method == 'GET':
return render_template('index.html')
ややこしいので書き直すとこんな感じです。
@app.route('/', methods=['POST','GET'])
def index():
"""ルートへのリクエストのハンドラー
Getリクエストの場合はindex.htmlを返す。POSTリクエストの場合はRequestBodyから画像と曲のテーマを受け取り、曲を生成したのち、view.htmlを返す
Returns:
GETリクエストの場合はindex.htmlが返される。
POSTリクエストの場合はview.htmlが返される。
"""
if request.method == 'POST':
music_theme = request.form.get('kind')
upload_image = Image.of(request.files.get('upfile'))
midi_music = MidiMusic.of(upload_image, music_theme)
mp3_music = MusicConverter.convert_to_mp3_from(midi_music)
return render_template('view.html',image=upload_image.get_path(),sound=mp3_music.get_path())
if request.method == 'GET':
return render_template('index.html')
docstringに従って書いたコメントやクラスごとの責務の見直し、命名の見直しをおこなったことで、かなり読みやすくなりました！これこそが真の設計書としてのコード！！！！
、、というのが大きな落とし穴で、その時自分が良いと思った設計・コーディングが設計書として本当に良いものだとは限りません。
現に、先程の改修前のコードも、これを書いた時点の自分は「綺麗なコード」として書いていたはずです。
が、実際はどうだったでしょうか？少なくともコードの作者(自分)は読みやすいとは感じませんでした、、、。
つまり、自分一人では設計書としてのコードを書くことは困難です。
故に、伝わるコードを書くためには
- リーダブルコードなどに書かれた先人の知恵
- 設計技法の知識
- 綺麗なコードを書くための意識
そして
- 多くの優秀なレビュワーに見てもらう
- 慢心しない
が必要です。
伝わるコードを書くって難しいですね。。
古くなるコード
タイトルを見て「モジュールの話ならdependabotがあるよ」って思った方もいるかもですが、ここでいう古くなるとは仕様が新しくなり、既存のドメインの概念や命名が変化することを指します。
先程の改修後のコードを例に説明します。
@app.route('/', methods=['POST','GET'])
def index():
"""ルートへのリクエストのハンドラー
Getリクエストの場合はindex.htmlを返す。POSTリクエストの場合はRequestBodyから画像と曲のテーマを受け取り、曲を生成したのち、view.htmlを返す
Returns:
GETリクエストの場合はindex.htmlが返される。
POSTリクエストの場合はview.htmlが返される。
"""
if request.method == 'POST':
music_theme = request.form.get('kind')
upload_image = Image.of(request.files.get('upfile'))
midi_music = MidiMusic.of(upload_image, music_theme)
mp3_music = MusicConverter.convert_to_mp3_from(midi_music)
return render_template('view.html',image=upload_image.get_path(),sound=mp3_music.get_path())
if request.method == 'GET':
return render_template('index.html')
このコードでは生成する曲のテーマのことを「music_theme」と呼び、人間向けに「テーマ」と呼んでいます。
ここで、諸事情で人間が「テーマ」を「曲の個性」などと呼び始めたとしましょう。
プロジェクトの仕様書も「テーマ」が「曲の個性」に書きかわり、会話に登場する用語としても「曲の個性」が使われるようになったら、先程のコード(あるいはこれよりより設計書と呼べるようなコード)はどうなるでしょう？
呼び方が変わっただけなので、プログラムとしてはちゃんと動作します。
が
なんと、読めなくなるんです、、。
正確には、当事者には読めるが第三者には読めないコードになります。
つまり、最初どんなに綺麗に書こうが、仕様の変化一つで伝わらないコードに逆戻りしてしまうんです、、。
故に、コードは仕様の変化に合わせてリファクタリングすべきですし、それ以前に最初から読みやすく、変更に強いコーディングが必要です。
コードを書くって責任重大ですね
(基本的に)コードは書いた通りに動作する
コードは思い通りに動作する訳ではありません。
(OSやOSS側に不具合がない限りは)書いた通りにしか動作しません。
そのため、次のコードはthreeと命名しているにも関わらず3を指しません。
int three = 2 * 2;
「何当たり前のこと言ってんだ？」と思った方もいるかもしれませんが、これが意外と難しいんです。
ソフトウェアを過信してはいけない
過去に起きた有名な大事故の一つにクラスターミッションというものがあります。
リンク先にもあるように、この事故は欧州宇宙機構が打ち上げたロケットが軌道に到達せず、自動起爆装置により爆発した、という事故です。
3億7000万ドル以上もの損害を出したこの事故の原因は、何と整数のオーバーフローによるシステムのシャットダウンでした
このように、ソフトウェアを過信すると意図せぬバグを引き起こす可能性が高くなります。
しかし、この事故がテストで防げたかどうかは定かではありませんが、少なくとも私たちはテストをすることで似たようなバグを極力防ぐことができます！
さらにさらに、CI/CDの導入によって幾つかのテストを自動化することができるのです！！文明の利器に感謝
、、とはいえ、カバレッジを意識したちゃんとしたテストを実施しなければ意味はないです。ちゃんとカバレッジを意識してテストケースを検討せずにテストを行っても、それは「思い通りに動けー！」と願ってるだけなのと何ら変わりません。
どんなに丁寧に作ろうがバグは発生しうるし、要件を理解した人が関わった適切なテストは必要なんです。ソフトウェアって難しいですね
何のために書いたかわからないテストコード
コードが思い通りに動いているかを自動で検討する手段として、テストコードによる自動テストは主流な手段です。
ここで、テストコードはコードを検証するコードなのですから、仕様の変化によりコードに変化が生じれば、当然テストコードにも改修が生じ得ます。
しかし、そのテストコードが何を検証するために、何を根拠にその閾値を用いて実施されたのかがわからなければ、テストコードを改修することは困難です。
例えば、次のような関数を検証するとします。
/**
* 3未満かどうかを返す関数
*/
public boolean isMinThree(int num) {
return num < 3;
}
そして、そのテストコードが以下のようだとします。
@Test
void isMinThreeFunctionTest() throws Exception() {
assertEquals(isMinThree(2), true);
}
@Test
void isMinThreeFunctionTest2() throws Exception() {
assertEquals(isMinThree(3), false);
}
このテストコードは何をしたかったテストコードなのでしょうか？色々想像はできますが、確かなことはわかりませんよね。。
また、これがあるクラスの膨大な量の関数の中のテストの一部だったらどうでしょう？とてもじゃないですがテストの意図を汲み取るのは不可能です
そこで、テストコードに意図を込めます。
@Nested
class isMinThreeメソッドは引数に与えた数字が3以下かどうかを返す {
@Test
void _2を渡したときtrueが返される() throws Exception() {
assertEquals(isMinThree(2), true);
}
@Test
void _3を渡した時falseが返される() throws Exception() {
assertEquals(isMinThree(3), false);
}
}
こうすることで、少なくとも何のために、何を意図して書いたテストなのかがわかるようになります！
(自分は先輩から共有していただいたt_wadaさんのTDDBCの講演動画を参考に書いています。)
これを全てのテストコードに実施することでテストコードを保守することが可能になりますが、それが意味のあるテストコードであることとは無関係です。
意味のあるテストケースを設計した上で、正しいテストコードを書き、かつ保守が可能な書き方で記述する必要があります。
テストコードって難しいですね
まとめ
本記事では自分の約半年強の実務経験から、仕事でコードを書くことの難しさについて振り返ってみました！
主に自分のためにコードを書いていた学生時代と比べると、品質のこと然り、コードを通じた情報伝達然り、仲間の開発効率然り、、、注意すべきことが増えて、コーディングというものが一気に複雑になったと感じています。が、それが楽しくもあります！
今後も慢心せず、楽しみながらも、コードの難しさ、およびソフトウェア作成の複雑さに立ち向かっていけたらと思います！
明日は@39yatabisさんの記事です！お楽しみに！
参考文献
- プリンシプル・オブ・プログラミング
- Wikipedia(https://ja.wikipedia.org/wiki/%E3%82%AF%E3%83%A9%E3%82%B9%E3%82%BF%E3%83%BC%E3%83%9F%E3%83%83%E3%82%B7%E3%83%A7%E3%83%B3)
- 知識ゼロから学ぶソフトウェアテスト
- TDD Boot Camp 2020 #1: 基調講演/ライブコーディング(https://www.youtube.com/watch?v=Q-FJ3XmFlT8)