2021-10-17 追加
弊社の Android 採用課題も公開されましたので、そのリンクを追加しました。
2020-05-18 追加
本日から弊社の採用課題がこちらに変更されました。これまではアプリをゼロから作成していただく課題でしたが、今後は既存のコードをリファクタリングしてもらう課題となりました。ただし我々が確認する項目はそれほど大きく変更するわけではありませんので、本記事の内容の多くは引き続き有効です。
ここ数ヶ月は、iOS のエンジニア採用のコードチェックにもよく参加していますので、そろそろ良さそうと思って、ここで私がコードチェックする時に一体何をチェックしているのかを共有し、皆さんの転職活動やキャリア設計に役に立てればと思います。
Disclaimer
この記事の内容はあくまで株式会社ゆめみの iOS エンジニア採用のものです。弊社以外の iOS エンジニア採用や、弊社でも iOS 以外のエンジニア採用にも必ずしも通用するとは限りません。また、弊社の Android エンジニア採用についてはぜひこちらの記事をご確認ください。
まずはコードチェックの課題を軽く紹介します。流石にここでいきなりネタバレするわけにはいかないのですが、とにかく簡単な課題です。最低限の仕様を満たすだけの内容でしたら、Xcode の初期テンプレートですでに入ったコードを除いて、自分で新規で書くコードなら 100 行も必要ないでしょう。皆さん転職活動するときに複数社のコードチェックを受けると思いますので、なるべくその負担を減らしたいと考えてこのような簡単な課題を出しているのです。（ちなみにそもそも課題を考案したのは私です）
しかしこんな簡単な課題でも、提出者のレベル感や、今までどんな開発スタイルでやってきたかなどの細かいことも意外と割り出せるのです。なぜなら課題は paiza のスキルチェックのようなアルゴリズムのチェックではなく、一つのアプリを作ってもらうからです。丸ごと一つのアプリを作ってもらうので、Xcode の使い方はもちろん、プログラムの設計や、OSS なライブラリーの利用なども見れますので、これは長い長いエンジニアライフで染み付いた習慣で、アルゴリズムのような短期間の猛勉強でなんとかなるものではありません。
レビュー内容
では私はコードチェックする側としていったい何を見ているのかを、これから実際私が出したレビューコメントを交えてご紹介します；
良かった点
人は皆褒められるのが好きです。私もそうです。なので私は基本的にレビューは全部褒める部分から始まります。最終的に NG くらうだろうか必ず良かった部分から始まります。
git の使い方に関するレビュー
- 細かいコミット粒度
- 適切なブランチ運用
- GitHub の PR 機能の利用
私はどんなプロジェクトでも、まず最初に確認するのは git のコミットグラフです。応募者は何を考えてコーディングしているのか、コードレビュアーの気持ちも考慮してくれているのか、などなどたくさんのことがわかります。実務をこなした方ならわかると思います。
Xcode を開く前に気づける工夫に関するレビュー
- README で概要の説明やリファレンスのリンクがきちんとかかれてある
- CocoaPods／Carthage による外部ライブラリーの利用
- アピールポイントや悩みを資料化してレビュアーとしてレビューしやすい
iOS アプリ開発で、多くの場合は Xcode プロジェクトファイル開いてそのまま実行すれば動く訳ではありません（もちろんそのまま動くものもありますが）。例えば CocoaPods や Carthage などを利用した場合は、先にそれらを通して利用するライブラリーをダウンロードしなくてはいけません。また実際アプリがどういう風に動くかとか、どういったアプローチで設計したかとか、何を参考にしたかとかも README で書いてもらえるとレビュアーとしてはとても助かります。
また CocoaPods や Carthage の利用も、もちろん必須という訳ではありませんが、うまく利用すれば書き手の苦労がかなり減ります。プログラマの三大美徳の一つに「怠惰」があります。コード量が多ければ多いほどいい訳ではありません。手を抜けるところをとことん抜くのがプログラマです。そのためにライブラリーの利用は非常に有効な手段です。更に言うと、ライブラリーをどれくらい有効に利用できるかというのはも、一人のプログラマの経験を間接的に評価できる指標の一つとも言えるでしょう。
テスト周りに関するレビュー
- 最低限の Test が書いてある
- ユニットテストも UI テストもきちんと作られてある
次はそろそろ Xcode 立ち上げて、コードもしくはアプリの動きを見ると思われるかもしれませんが、私は次に気になるところはテストです。もちろんテストがどこまでカバーしているかは応募者によってまちまちですが、少なくとも一つでもあれば「この人はテストを書くと言う意識がある」のがわかります。もちろんテストが豊富なほど、クォリティー意識が高いと見受けられるので、私からの評価は高くなります。
アプリの UI と使い勝手に関するレビュー
- 見やすい表示
- シンプルながら必要な機能が手軽にアクセスできる UI
- シンプルながらもちょっとした可愛らしさを備えた画面デザイン
- 使いやすい UI
- デザイン、アニメーションの工夫
- 非常に気合が入ってなおかつおしゃれで使いやすい UI！
次はいよいよ実際アプリを動かしてどんな風に表示されるか、どんな風に操作されるかを見ます。私は具体的なコードよりも先にはアプリの動きを見ます。どんなに優雅なコードを書いても、それが要件とおりに動かないなら無意味だからです。また、アプリエンジニアとしても、最低限の UI/UX への気遣いも必要だと思います。別にすごいおしゃれな UI を求めるわけではありませんが、最低限利用者がストレスなく利用できる程度の UI は必要だと思います。
ちなみに今までで、何人かの応募者は確かにとてもおしゃれな UI を実装したことがありました。更にうち一人は「この人マジかよ…この気合の入れ方半端ない」と思わせてくれるめちゃくちゃすごい UI をぶち込んでくれました。もちろんその方は今弊社に入社しており、我々の仲間として一緒にいいアプリ作りに力を発揮してくれています。
要件外の機能の実装に関するレビュー
- UX に気を配って〇〇機能を実装した
- 〇〇画面を作った
iOS アプリのコードチェックとして出している課題は本当にとても最低限なことしか要件を書いていないので、ユーザの実際の利用においてもっと便利な機能を実装してくれる応募者もたまにいます。このような要件通りに実装するだけでなく、アプリとしての本当の目的を着目してアプリエンジニアとしての工夫や提案力は、私個人としても非常に高い評価を与えたいと考えておりますし、弊社の業務上も大変貴重な能力だと私は考えます。弊社は（基本的に）自社サービスを持たずに、お客様の需要に応じてアプリを開発していますが、それは言われた通りのことをするだけでなく、より良い製品を共に作っていこうと言う精神は非常に大事だと思います。
設計や保守性周りに関するレビュー
- Xcode のプロジェクトツリー構造が綺麗に作られている
- MVC/MVP/MVVM/Redux による適切な設計
- 読みやすさに気を配って細かくメソッドを切り出している
- 最低限の責務分割ができている
- 比較的に簡潔で無駄が少ないコーディングなので読みやすい
- UI 層や Domain 層などがきちんと別れてあり、責務が分離されている
アプリは一度開発したら終わりと言うようなものではなく、サービスの存続にもよりますが長い間保守し続けなくてはならないものです。しかもその保守の間に、開発者の入れ替えでいつの間にか初期開発の人が誰一人残っていないこともよくあります。更に言うと弊社は「有給取り放題」と言う制度が設けており、いつ誰が急に休まれても、代わりになるメンバーが確保できると言う意味で属人化の解消は必須です。そのためアプリが適切な設計で責務をきちんと分離しているか、ソースコードが初見の人でもちゃんと何がやりたいかを読み取れるか、といった保守性に関わる部分が非常に大事です。
最初にも言いましたが、こんな簡単な課題でも、意外とレベル感や今までどんな開発スタイルでやってきたかなどの細かいことが割り出せるのです。その原因の多くはこの部分ですね、これは長年の開発経験で身に付くスキルであり、とても転活直前にとりあえずコンテスト本を頑張って丸暗記してできるようなものではありませんし、更にこれでこの人が今までどんな環境で開発してきたか、いまだに 10 年前のレガシー環境に生きているのか、それともちゃんと定期的にキャッチアップしているのかもわかってきます。
ちなみに、私の場合、この部分のレビューでコケた人は、ほぼ 100％ の確率で NG をくらいます。課題が簡単な代わりに、保守性をとことん重要視しています。
画面レイアウトに関するレビュー
- 画面レイアウトにサイズクラスを活用して、シチュエーションに応じて柔軟なレイアウトを設定している
- Storyboard と XIB で正しくレイアウトを設定している
- 適切な Auto Layout 設定
- コードレイアウトで概ね正しく行われている
私自身は Auto Layout および Storyboard が大嫌いですが、別に中途採用においてこれらを使ったら不合格になるわけではありません。Auto Layout でもコードレイアウトでも、きちんとレイアウトサイクルに沿って正しくレイアウトできていればいいです。
その他コード面においての工夫に関するレビュー
- UIColor の Semantic 化した利用
- 必要に応じてソースコードドキュメントが書かれてある
- イレギュラーな操作にコメントで理由を入れてる
- SwiftLint を導入し、ソースコードの品質管理がされている
- R.swift を導入し、ハードコーディングを避けている
- 環境変数の定義
これらはある意味保守性にも関係していますが、設計や可読性などのような必須な要件と比べると、これらは「プラスアルファ」的なものだと思います。ソースコードは必ず読みやすくなくてはいけませんが、SwiftLint の導入等は必須ではありませんから。でもこれらの工夫を施すと、「この人は本当にプロジェクトの保守性を気にしている」という印象が強くなります。
もちろんこれらはあくまでプラスアルファなので、可読性や責務の分割もできてなかったら、これらの評価ポイントで頑張っても基本的に合格は難しいです。
改善点
もちろん良かったところだけ挙げても次のステップに繋がらないので、次は良くなかったところを挙げなければならないのです。でもそれは「良くなかった点」ではなく「改善点」だと前向きに捉えてほしいので、私はこの部分を「改善点」と名付けています。
git の使い方に関するレビュー
- .gitignore がない
- .xcodeproj ファイル自体が完全に ignore されてるので動作確認ができなかった
- git の init commit がすでに色々実装しちゃってるので、実装の考え方が見えなかった
- コミット粒度が割と雑
- コミットメッセージが全て空なのでわかりにくい
- まだ git のブランチ運用ができていない
git の利用で褒められる人もいれば、もちろんそうはいかなかった人もいます。git がうまく使えてるかどうかはその人の開発経験を間接的に見せてるだけでなく、開発するときに何を考えて実装しているのか、レビュアーがコードレビューするとき読みやすいかなどの、実務での開発で見なくてはいけないものもたくさん見れますので、是非とも git の使い方に気をつけていただきたい所存です。
また、今までの経験で、一人 .gitignore で *.xcodeproj ファイル自体を無視しちゃったため、そもそも Xcode で開けず、動作確認ができなかったというハプニングもありました。.gitignore の構成も是非とも気をつけていただきたいです。何を入れればいいかわからなかったら、こんな優れたサービスもあるから、是非ともご活用ください。
ライブラリーの利用に関するレビュー
- 3rd パーティー製ライブラリを利用してコードを減らそう
- Carthage のフレームワークは Embeded にしているが、それではシミュレーター用のバイナリも入ってしまうため本番審査だと通らない
課題の要件では特にライブラリーの利用は必須ではなく、あくまで「ライブラリーの利用はオープンソースのものに限る」としているだけですが、それでも全くライブラリーを利用せずに全て自分で組み込むのはまあまあの手間が発生します。自分の手間を減らすためにも積極的にライブラリーを利用しましょう。また、利用するときはツールの説明を読み、正しく利用しましょう。
そもそもの課題要件に適していない環境に関するレビュー
- ビルドシステムがレガシーシステムになっている
- 可能ならやはり Swift で提出していただきたかった…
- Swift のバージョンが最新（5.0）ではない
たまにいるんですよね、課題の要件としては原則として最新の安定版の利用をお願いしているにも関わらず古い環境で提出した人もいます。これに関してはそこまで厳しく見てはいませんが可能ならやはり最新のもので提出していただきたいです。
ちなみに面接で尋ねたら「私物の PC が古くて最新の Xcode が入らない」という理由が多いです。また一人はずっと Objective-C しかやってこなかったため、Swiftが力不足だと考えて Objective-C で提出したそうです。
テスト周りに関するレビュー
- テストがない
- テストが外部に依存しているので、テスト対象の分離ができていない
良かった点の解説でも言いました通り、どんなに単純なテストコードでも、少なくとも「自動テストでクォリティを確保する」という意識が読み取れるので、とても大事です。是非テストを組み込んでください。
ただ一つ気をつけないといけないのは、アプリコードのテストで保証しているのはあくまでアプリのスコープのみで、そこにアプリコード自身以外の何かへの依存を持ち込むのは良くないです。
アプリの UI と使い勝手に関するレビュー
- 実機では〇〇ボタンがキーボードに隠されて押せなくなっている
- 画面遷移で戻るのアニメーションが新規画面への遷移になっている
- 画面レイアウトが多様な解像度を考慮していない
- 入力窓と出力窓がそれぞれ「高さ」でレイアウト設定されているので、例えば iPhone SE などの小さい端末だと画面一部が見切れてしまうことがある
- HIG 的には、ポジティブ動作の変換ボタンを右に置いた方が自然で、右手で操作するときタップしやすい
- 〇〇ボタンが一番上にあるので、実際の利用では微妙にタップしにくい
- キーボードの上に何も起こらない
Doneボタンがあるが、その割には明示的な〇〇ボタンがないため、動作するのに UX 的にちょっと苦労した
前にも言いました通り、アプリエンジニアとはいえ、ユーザ視点から見た最低限の使いやすさへの配慮が必要です。基本的に私から「ダサい」というようなレビューは出しません、見た目が美しいかどうかはデザイナーの仕事だからです。しかし「このボタン配置では使いにくい」「このアニメーションでは遷移がわかりにくい」と言った使い勝手のダメ出しや、アップルの HIG の違反の指摘はあり得ます。iOS アプリ開発者なので、HIG は熟知しておきましょう。
設計や保守性周りに関するレビュー
-
ViewController2という名前がかなりわかりにくい - ビジネスロジックが VC に実装されて Fat になっている
- FatVC ではないが、Xcode のディレクトリーの分け方としてはビジネスロジックの実装が UI に依存した形になっている
- ViewController に不必要なプロパティーが大量に残っている
- ハードコーディングが多い
- プロパティーやメソッドのアクセス制限（
private varなど）がない - コーディング規約としての一貫性がない（
{の前や:の後にスペースが空いたり空いてなかったりする） - コードでレイアウト決める際に、View がデバイス全画面であることとか、親ビューの origin が (0,0) とかの暗黙な依存が発生している
- 行を空けることがほとんどないため、ソースコードがほとんどかたまっててあまり読みやすくなかった
- エラーハンドリングが全くない
- Base クラスなど のOOP の影響が強く、Swift の特徴である POP が活用できていない
- メソッドやプロパティーのドキュメントコメントは問題ないが、実装の通常のコメントもかなり多くてノイズにもなっている
この部分に関する指摘が一番多いですかね。ここにあげてる例もほんの一部にすぎません。
前にも言いました通り、課題は簡単ですがその代わりに保守性をとことん重要視しています。軽微なもの（例えばほんの少しのハードコーディングなど）だけならもちろんそれだけで落とすことはないですが、責務が分離されてなかったり、命名がわかりにくかったり、副作用が横行したりするような保守性が非常に悪いコードは確実に NG をくらいますので、ぜひ気をつけていただきたいところです。
画面レイアウトに関するレビュー
- レイアウトのコード自体もそんなに読みやすくはない
- 子ビューのレイアウト設定に親ビューの
frameを使っているが、boundsを使うべき - 追加ビューのレイアウトが
frameの設定のみで、制約もしくはレイアウトサイクルによるレイアウト設定がない
まあここでコケる人はそんなに多くいません、なぜなら多くの人は普通に大人しく Storyboard を使っているからです。その場合、Storyboard はビルド前にちゃんとエラーやワーニングを出してくれるので、変なことさえしない限りここで失敗することはあまりありません。
ところがどうしても Storyboard だけで作れない UI を作ってしまったら、frame と bounds の違いがわかっていないや、レイアウトサイクルを意識していないと言ったようなことが目立ち始めます。ビルド時に保証してくれるものがないですからね。
まあ今後は SwiftUI に移行したら、こんな面倒くさいことを意識しなくて済むので、基本的にここで失敗しても NG を出すことはあまりありませんが。
バグに関するレビュー
- VC が複数のインスタンスが生成して破棄されず、メモリリークになっている
- キャプチャーリストがないためクロージャーでもメモリリークが発生している
- Delegate で強参照を利用しているため循環参照が発生している
- 非同期で取得した結果をまだ画面に反映できてない
- 一回だけ設定すればいいはずの項目をライフサイクルにおいて何度も行うメソッド（
draw(_:)）で処理させている - 結果画面の結果表示が編集可能になっている
やはり非同期処理が入るので、どうしてもクロージャーが使いやすかったりしますが、わかりやすい症状もないのでキャプチャーリストを忘れてメモリリークが発生する人結構いますね。そして同じメモリリークにつながりますが、Delegate で強参照のまま使ってメモリリークになる人もいます。
そしてメモリリーク系以外に、ライフサイクルを理解せずに一回だけ行うべきの処理をなんども呼び出されるメソッド内に入れる人もいます。こういうビルド時に教えてくれないし、実行時も目立つ不具合があまりない、さらに言うと再現するユニットテストも割と作りにくいバグにもぜひ注意していただきたいです。簡単な課題なのでコード量が比較的に少なくまだ気付きやすいですが、大規模な開発になると徐々に気付きにくくなってきます。
あとがき
以上、私がコードチェックする際にいつもどこを見ているのか、と言う簡単な紹介でした。また、全員に送ってるわけではありませんが、時々細かい NG と言うわけではないがちょっと気になるところ（（例えばヨーダ記法が 1 箇所だけあるとか）があったり、もしくは課題全体の完成度について思ったこと（例えば微妙にコーディングスタイルがちょっと古そうに見えるとか）にあったらそれらも併せてレビューに付け加えることがあります。これから転職する予定の皆さんのご参考になれば幸いです。