【Swift】人生初めてのコードレビューを体験してみた

Swift・Xcode プログラミング

 

MediaPlayerの使い方を学ぶ名目で音楽プレイヤーもどきのアプリを作ったのですが、

 

（自分のコードって周りから見てどうなんだろう？）

 

と疑問が浮かびました。

 

僕の周りにはプログラミング仲間と呼べるような人など1人もおらず、TwitterやSlackのコミュニティに参加してはいたものの、自分を評価してもらうようなことをしてもらったことがあります。

 

というわけで、自分のコード品質を評価してもらうために、Slackのコミュニティで、

 

どなたか、コードレビューをお願いできませんか？

Swiftで２つ目の音楽プレイヤーアプリを作りました。

ただ、自分のコードを客観的に評価してもらったことがなく、今のうちに自分のコードを綺麗にしておきたく、お願いした理由です。

レビュー結果は1週間後でも、厳しいコメントを述べても構いません。

お時間のある方でよろしいので、よろしくお願いします。

 

と質問を投げたところ、すんなりと

「やります」

と数十分で返事がもらえました。

  

数日後にレビュー結果が出て、僕の反省点が露呈したので、備忘録として書いていきます。

  

コードレビューの結果  

 

結果からわかった反省点は、

 

  1. 変数名に大文字を用いてない
  2. 変数の役割を変数名で示していない
  3. Yes/Noなどの0/1分岐を数字で処理している

 

の３つです。

 

反省点１：変数名に大文字を用いてない

 

僕はたまに大文字を打たないことがあります。

 

理由は、 

  • いちいちShiftキーを押す必要がある
  • 英単語で変数を区別できる 

からです。

 

しかし、レビューには、

 

単語ごとに大文字で始めると見やすくなります。

 

というお言葉をいただいて、大文字を含めれば可読性が上がると理解しました。

 

例えば、 

@IBOutlet weak var songname: UILabel!
//
@IBOutlet weak var artistname: UILabel!
//
var currenttime = 0.0
//
let notificationcenter = NotificationCenter.default
//
func updatesong(mediaItem: MPMediaItem) {

と大文字を用いてない箇所が多く見受けられます。

 

上記のsongnameの場合、

Song + Name ：曲の名前

という意味が込められています。

 

なので、songとnameを区別するために 

songName

のように英単語の頭文字を大文字にすれば、songとnameを区別できます。

 

他の変数も、 

artistName 

とすれば、わかりやすくなります。

 

よくよく見れば、

currenttime

ってtが２つ連続で繋がっているので読みにくいですね..。

 

反省点２：変数の役割を変数名で示していない

 

プログラミングのアプリやゲームの開発では、何千ものコードを書くことになります。

 

なので、エラーや不具合が起きて全体のコードを見直すときに、どの箇所がどの機能を果たしているかを示せるように変数名をわかりやすくしなければなりません。

 

もし”SNSへ共有する画面を出す”機能を果たす関数の名前が

 abc

だったら、何の関数なのかわかりませんよね（笑）?

 

迅速にエラーを解決するどころか、余計にエラー箇所を探す手間が増え、貴重な時間が無駄になります。

 

僕のコードを例にすると、

timeinterval

は時間のデータを扱う変数だが、”何の”timeintervalかを示していません。

 

せめてsongTimeinterevalと命名するべきでした..。

 

反省点３：Yes/Noなどの0/1分岐を数字で処理している

 

数字や文字列などの明確で明らかな値ならまだしも、

曲が再生中/停止中か？

という判断するためのメソッドもクラスもない場合は、

 

0：停止中

1：再生中

 

と数字でスイッチのように分岐させていました。

 

しかし、0が停止中で1が再生中なんていうルールなんて、コードを読み込まなければ第3者からはわかりません。

 

今回のコードでは、

var playorpause = 0

と記述しましたが、

 

変数名：isPlaying

値：Bool（true：再生中、false：停止中）

 

というコメントをいただきました。

 

上記の場合、変数名が”isPlaying”(再生中)を示し、

 

true：当てはまる

fasel：当てはまらない

 

と、変数名とBool値がマッチして、変数と値の役割をわかりやすくなります。

 

他にも、 

var flag = 0

も曲が初めて再生されたかを示す変数なので、

var hasBeenPlayed = false

と変更すればマシになりますね。

 

まとめ

コードレビューでの反省点は、

 

変数名に大文字を使え

変数に適した英単語を変数名に含めろ

条件分岐をBool値で実装せよ

 

でした。

 

初めてのコードレビューでしたが、迅速で丁寧なレビューでとても勉強になりました。

 

客観的にコードを評価してもらうには、スクールに通わなくても、TwitterやSlackでお願いすれば、意外とすんなり評価してもらえます。

 

TwitterのDMよりも、コミュニティの方でメンバーの帰属意識や連帯感がありますから、Twitterでよりも頼みやすいかもしれません。

 

次はアプリレビューをお願いしたいですね。