Help us understand the problem. What is going on with this article?

可読性と変更容易性のため、気をつけていること

コメント頂いた件について追記しました。 2020/02/24 22:17
パイプライン演算子について追記しました。 2020/02/26 1:59
コメントの返信は終了しました。 2020/02/26 11:46
タイトルを変更しました。旧タイトル「可動性のためなら改行してくれ」 2020/02/27 09:38

この記事の内容は可読性と変更容易性の向上のための 1つの方法を記載しています。
可読性の感じ方には個人差があります。

ソースコードで一番大事なのは、 速度よりも可読性 です。
少しくらい速度が上がるからといって、可読性を損なうのは基本的には避けるべきで、
止む得ない事情がある時のみであると、自分は思ってます。

・可読性を損なわない高速化は行う
・可読性を損なう高速化の場合は正しいクラス設計で開発し、動きが保証されてから最適化する

特定の文字列をもらった後に、決まった文字列を後方につけ、暗号化してストレージに保存を行い、
ストレージ保存した際にResultを return する。
パスワードなどの文字列をそのまま暗号化せず Salt をつけ、保存するような処理です。
※とりあえずの例です。

読みにくい例
savePassword (password) {
  return Storage.save(encrypt( password + "Jga2favSaFaw" ));
}

このような例は良くないです。
動作自体は別に問題ありませんし(※)、よく見ればどういう順序で処理されているかわかります。
ただ、解読に時間がかかります。

※追記: 本来の暗号化としては、Salt は encrypt の関数の中だけに存在している方が望ましいです。 今回は例として、呼び出し元の関数から渡すやり方にしています

それはなぜかというと基本的に人間は、前から処理を読むからです。
※個人差はあるようです
return, Storage, Save, encrypt, password, + "Jga2favSaFaw" とここまで読んで初めて、
まずは password + "Jga2favSaFaw" が動いてOKと理解できます。

理解の過程を表であらわします。

理解できたところ 脳内メモリ 他に考えること   
1 password + "Jga2favSaFaw" return, Storage, Save, encrypt "Jga2favSaFaw"てなに・・?
2 encrypt( password + "Jga2favSaFaw" ) return, Storage, Save
3 Storage.save(encrypt( password + "Jga2favSaFaw" )) return
4 return Storage.save(encrypt( password + "Jga2favSaFaw" )); なし

このような過程を経て、理解できるようになります。
通常は他のメソッドも一緒に考えながら読むため、更に脳内メモリを消費していることになります。
これにより、 「あれ・・?これ、結局どうなってんだっけ・・?」 ってなってしまい、
ソースコードの理解が進まないようになってしまいます。

ではどうするか

以下のように書き直します。

:読みやすい例
SALT="Jga2favSaFaw";

savePassword (password) {
  const savingText = password + SALT;
  const encryptedSavingText = encrypt(savingText);
  const saveResult = Storage.save(encryptedSavingText);
  return saveResult;
}

こういう書き方に変えることにより、脳内メモリの消費量を少ないまま、
ソースコードを読むことができます。

理解の過程を表であらわします。

理解できたところ 脳内メモリ 他に考えること
1 SALT="Jga2favSaFaw"; SALT
2 const savingText = password + SALT; savingText
3 const encryptedSavingText = encrypt(savingText); encryptedSavingText 暗号化している
4 const saveResult = Storage.save(encryptedSavingText); saveResult 保存している
5 return saveResult; なし 戻り値は saveResult なのか

脳内メモリの消費が少ないまま、理解ができるようになります。
こうすることで理解が楽になり、可読性が良いということになります。
また変数名により、処理を終えたもの一体何者か、ということが説明できています。
重要な return値も最終行を見るだけで、saveResult が返っているってことがわかります。

メンテナンス性も高い

saveResult がマイナス値であれば、別の処理を追加したいなどがあった場合に、
既存の処理をそのままに間に処理を増やすことができます。

savePassword (password) {
  const savingText = password + SALT;
  const encryptedSavingText = encrypt(savingText);
  const saveResult = Storage.save(encryptedSavingText);

  // saveResult を使った追加処理がそのまま書ける

  return saveResult;
}

デメリット

デメリットはまったくないかといえば、少しあります。
変数を作った際に多少のコストがかかるところです。

ただ、そのレベルで問題になることは極端にメモリが少ない組み込み系など一部のコードです

更にいうと、可読性が悪いことによって、明らかに無駄な処理を書いてしまっていることの方がほとんどです。
なので、速度の面でも可読性を優先することは意義があります。

まとめ

・可読性の維持を最優先としよう。
・可動性を損なう最適化はやめよう ※組み込み系は除く
・可読性を維持できる最適化はやった方がいいです

追記 2020/02/24 22:17

他の方法がコメントで提示されましたが、自分がやらなかった理由を書いておこうと思います。
好みの問題が大きく関わってくるとは思います。

return のために変数を用意するのは必要ないのでは?

return Storage.save(encryptedPassword);

このように書いた方がスムーズでは?という指摘がありました。
やらなかった理由は、 Storage.save の戻り値の型がわからないからです。

save の戻り値は isSuccess のような真偽値なのか、 isFailed なのか、
saveState という Enum値なのか、 resultNo という特殊なものなのかは、わかりません。

save のメソッドを見に行かなくてもわかるように、変数名を決めて定義しています。
なので、上の例では、 saveResult というよくわからない名前にしてしまいましたが、
本当は isSuccess など明示的な名前にすべきでした。

また、isSuccess の戻り値によって、追加の処理を書くことになった場合、
行追加のみで対応が可能になります。 return Storage.save(encryptedPassword); と書くと、
一度行をバラす必要が出てきます。

1行でも右から読めばすぐわかるのでは?

読み方の問題では?という指摘がありました。
たまたま引数が1つの処理の連続だったため、なんとか読めるのでは?と思いました。

引数が1つの間は大丈夫かもしれませんが、
仕様変更で、引数が複数に追加された場合や、引数として、配列があったりで改行が挟まるようになった場合などでは、
破綻するかと思います。

return Storage.save(encrypt( password + "Jga2favSaFaw", EncryptType.AES, KeyLength.256,[
  'aaaa',
  'vvv',
  'dddd',
] ), StorageType.Local, SaveTime.infinity );

その状況によって書き方が変わることはそもそもないよう、初めから行を分けておくことで、
未来の可読性に備えているっていう理由があったりします。

関数型のように変数を保持せず書くやり方の方が可読性が良いのでは?

関数の中で1行目に return を書くやり方です。

savePassword (password) {
  return Storage.save(
    encrypt(password + "Jga2favSaFaw")
  );
}

この場合、可読性に置いては問題はないかなぁと思います。
自分としてはメンテナンス性の問題があります。

return Storage.save(encryptedPassword); の例でも書いた、save の戻り値の型の問題

もし、save の戻り値によって if の判定処理が必要になった場合、バラす必要が出てきてしまいます。
処理をバラすということは、その処理について再度理解をしないといけなくなってしまいます。

一度変数を作ると後で使われる場合のことを考えなくてはいけないのでは?

これは実際使われる時になった場合に考えれば良いかと思います。
また、覚えとかないといけなくなったとまで、行数が離れた場合は、おそらくメソッド分けしていないことが問題であり、
別の問題になっているはずです。

子供のメソッドを追加して、現在のメソッドから呼んであげるように分割すれば問題はないです。

パイプライン演算子で書けば良いのでは?

記述としては、あまりにもシンプルに書け優秀なやり方だと思いますので、
使える言語では一部採用しても良いかと思います。

ただ、パイプライン演算子は引数が1つの時のみ有効なものだったかと思います。
複数になった場合には使えなくなります。
基本ベースはこの記事の書き方で考え、パイプライン演算子はメインにせず、
使える時には使うっていう程度が望ましいかなと思います。

変数を置くところをある程度、この間には処理が挟まることは基本的にないだろう、
と思えるところでは、まとめるのは良いかと思います。

savePassword (password) {
  const encryptedSavingText = (password + SALT) |> encrypt;
  const saveResult = Storage.save(encryptedSavingText);

  // saveResult を使った追加処理がそのまま書ける

  return saveResult;
}

dd0125
Android Java, Objective-C, Ruby, PHP, Java, Java Script, JSX, Go, C#, VB.net など色々プログラム書いてます。
yumemi
みんなが知ってるあのサービス、実はゆめみが作ってます。スマホアプリ/Webサービスの企画・UX/UI設計、開発運用。Swift, Kotlin, PHP, Vue.js, React.js, Node.js, AWS等エンジニア・クリエイターの会社です。Twitterで情報配信中https://twitter.com/yumemiinc
http://www.yumemi.co.jp
Why not register and get more from Qiita?
  1. We will deliver articles that match you
    By following users and tags, you can catch up information on technical fields that you are interested in as a whole
  2. you can read useful information later efficiently
    By "stocking" the articles you like, you can search right away
Comments
No comments
Sign up for free and join this conversation.
If you already have a Qiita account
Why do not you register as a user and use Qiita more conveniently?
You need to log in to use this function. Qiita can be used more conveniently after logging in.
You seem to be reading articles frequently this month. Qiita can be used more conveniently after logging in.
  1. We will deliver articles that match you
    By following users and tags, you can catch up information on technical fields that you are interested in as a whole
  2. you can read useful information later efficiently
    By "stocking" the articles you like, you can search right away
ユーザーは見つかりませんでした