###実現したい関数
入力された文字列に対して、その行数を返す関数を作りたいのですが、
その際最小値と最大値を設定する必要があります。
js
1function(text) { 2 const minHeight = 5; 3 const maxHeight = 10; 4 const textHeight = text.match(/\n/g) ? text.match(/\n/g).length + 1 : 0; 5 return Math.max(Math.min(textHeight, maxHeight), minHeight);; 6 }
書いてみたのですが、もうちょっとわかりやすく書けないかな〜と思っています。
何かもっといい書き方ありませんか?
コードの短さよりもわかりやすさを優先とします。
気になる質問をクリップする
クリップした質問は、後からいつでもMYページで確認できます。
またクリップした質問に回答があった際、通知やメールを受け取ることができます。
バッドをするには、ログインかつ
こちらの条件を満たす必要があります。
回答3件
0
構造化プログラミング(structured programming)
構造化プログラミング(structured programming)と呼ばれる思想があり、その原則に「関数は単一の入口と単一の出口を持つべきである」というものがあります。
JavaScript
1'use strict'; 2function getHeightClamped (text) { // 仮引数 (単一の入口) 3 const minHeight = 5; 4 const maxHeight = 10; 5 const textHeight = text.split("\n").length; // 返り値となる変数を作る 6 7 if (textHeight < minHeight) { 8 textHeight = minHeight; // 返り値に最小値を代入する 9 } else if (textHeight > maxHeight) { 10 textHeight = maxHeight; // 返り値に最大値を代入する 11 } 12 13 return textHeight; // 返り値を返す (return 節を一度だけ使う事で単一の出口となる) 14}
この原則の利点として、アルゴリズムを書きだした時にすっきりした構造となることが挙げられます。
入口/出口が複数あると、考えるパターンが複雑化してしまい、人間の目で見て追いきれない事があります。
しかしながら、構造化プログラミングには入口/出口が狭い都合上、コードの階層が深くなってしまう欠点があり、最近の考え方としては「例外処理は値を先に返す事で追い出してしまい、後続コードを正常系処理のみにする事ですっきりしたアルゴリズムにしよう」が主流なようです。
実際にJavaScriptの基本仕様である ECMAScript でも同様のアルゴリズムが組まれています。
一例として、ECMAScript 5.1 規定の Array.isArray
の処理を読んでみましょう。
- If Type(arg) is not Object, return false.
- If the value of the [[Class]] internal property of arg is "Array", then return true.
- Return false.
- 引数
arg
が Object 型でない場合はfalse
を返す - 引数
arg
の内部プロパティ[[Class]]
が"Array"
ならば、true
を返す false
を返す
1., 2., 3. のstepで**「そうでなければ(else)」の文言がないのは、前stepで値を返す事で例外処理が終わっている為**です。
互換コード(Polyfillコード)は次のように書けます。
JavaScript
1Object.defineProperty(Array, 'isArray', {writable: true, enumerable: false, configurable: true, 2 value: function isArray (arg) { // 1. If Type(arg) is not Object, return false. 3 4 if (Object(arg) !== arg) { 5 return false; 6 } 7 8 if (Object.prototype.toString.call(arg) === '[object Array]') { // 2. If the value of the [[Class]] internal property of arg is "Array", then return true. 9 10 return true; 11 } 12 13 return false; // 3. Return false. 14 } 15});
このコードは「入口が一つ、出口は複数」ですが、正常系処理に限っては「入口が一つ、出口が一つ」とする事で構造化プログラミングの理念の則ってすっきりしたコードを書けます。
余談ですが、入口を一つにする原則は、「参照透過性が成り立つコード」を組む時にも通用する考え方となります。
参照透過性が成り立たないコードは「引数とは別に入口がある」と考えられるからです。
複数の出口を持つコード
私は基本的に汎用性重視でコードを組みます。
- 最小値/最大値は引数で指定します(オプション)
- 引数無しは無制限です
JavaScript
1'use strict'; 2function getHeightClamped (string /* [minHeight [, maxHeight] */) { 3 const argumentsLength = arguments.length, 4 minHeight = argumentsLength > 1 ? Number(arguments[1]) : 1, 5 maxHeight = argumentsLength > 2 ? Number(arguments[2]) : 1/0, 6 stringHeight = string.split(/\r\n|[\n\r]/).length; 7 8 if (stringHeight < minHeight) { 9 return minHeight; 10 } 11 12 if (stringHeight > maxHeight) { 13 return maxHeight; 14 } 15 16 return stringHeight; 17} 18 19console.log(getHeightClamped('a\r\nb\nc\rd\ne')); // 5 (\r\n, \r, \n の3タイプの改行コードを全て認識する、第二引数以降省略したら無制限) 20console.log(getHeightClamped('a\nb\nc\nd\ne', 1, 3)); // 3 (最小値: 1 / 最大値: 3) 21console.log(getHeightClamped('a\nb', 3)); // 3 (最小値: 2 / 最大値: 無制限)
return
節が複数存在する事から、このコードは「構造化プログラミング」の原則に反しています。
このコードは「例外処理系」と「正常処理系」を分割する事に利点があると考える為、else-if 節で繋がず、空行を開ける事で「それぞれの例外処理」を細かく分けるように書いています。
更新履歴
- 2017/10/29 16:37 「構造化プログラミング」の節を追記。それに伴い、「複数の出口を持つコード」の節に補足説明を追記。
Re: nabettu さん
投稿2017/10/29 06:11
編集2017/10/29 12:41総合スコア18162
0
別にそれでも良いと思いますが、より愚直に計算するとこうでしょうか。
lang
1function getHeightClamped(text) { 2 const minHeight = 5; 3 const maxHeight = 10; 4 const textHeight = text.split("\n").length; 5 if (textHeight < minHeight) { 6 return minHeight; 7 } else if (textHeight > maxHeight) { 8 return maxHeight; 9 } else { 10 return textHeight; 11 } 12}
投稿2017/10/28 15:08
総合スコア2551
バッドをするには、ログインかつ
こちらの条件を満たす必要があります。
2017/10/29 06:21 編集
2017/10/29 06:25
2017/10/29 07:50 編集
2017/10/29 08:22
0
ベストアンサー
別の視点から。
- 関数は細かく分ける。一つの関数に一つのロジックしか書かない。
- 関数に汎用性を持たせる。汎用的な関数から特殊化していく。
という考えがあります。ただし、汎用化すると言っても今必要な機能だけを実装していかないとYAGNIに反して無駄な処理が多くなりますので、注意が必要です。
JavaScript
1// 範囲内に無い場合はその範囲に収める 2function correctRange(value, { min = -Infinity, max = Infinity }) { 3 if (min > max) { 4 throw new Error(`min is over than max: min ${min}, max: ${max}`); 5 } 6 if (value < min) { 7 return min; 8 } 9 if (value > max) { 10 return max; 11 } 12 return value; 13} 14 15// 文字列中の文字の数を数える 16function countChar(str, c) { 17 return Array.from(str).filter(s => s === c).length; 18} 19 20// 文字列の行数を数える 21function countRow(str) { 22 const eolCount = countChar(str, '\n'); 23 const eolTemrinator = str.length === 0 || str.endsWith('\n'); 24 const lastLineCount = eolTemrinator ? 0 : 1; 25 return eolCount + lastLineCount; 26} 27 28// 文字列中の文字の数を数え、範囲内に収める 29function countRowCorrectedRange(str, range) { 30 return correctRange(countRow(str), range); 31} 32 33// 文字列中の文字の数を数え、5から10に収める 34function countRowFrom5To10(str) { 35 return countRowCorrectedRange(str, { min: 5, max: 10 }); 36} 37
上は一例に過ぎません。これより良い分け方があるかも知れません。このように書いた場合の利点は次のようなことです。
- 再利用しやすい。例えば、5から10ではなく20から50が必要になっても、
countRowFrom5To10
を1行変えた(といっても、元々1行だけですが)だけの関数を用意すれば良い事になります。 - 変更しやすい。例えば、最後が改行で終わらない文字列の場合にその行も1行も数えるかどうかが問題になるとき、
countRow
のロジックだけに集中して修正することができます。 - 改良しやすい。例えば、文字数をカウントする
countChar
のパフォーマンスが良くない(実際に上のコードはあまり良くないと思います)として、この高速化を考えたとき、countChar
の中身だけに集中して改良することができます。
たかが4行のコードでここまですべきかどうかは時と場合によると思いますが、保守性を重視し、それぞれについて単体テストも用意するのであれば、間違った選択では無いと思います。
投稿2017/10/29 12:27
総合スコア21735
バッドをするには、ログインかつ
こちらの条件を満たす必要があります。
退会済みユーザー
2017/10/29 13:25
あなたの回答
tips
太字
斜体
打ち消し線
見出し
引用テキストの挿入
コードの挿入
リンクの挿入
リストの挿入
番号リストの挿入
表の挿入
水平線の挿入
プレビュー
質問の解決につながる回答をしましょう。 サンプルコードなど、より具体的な説明があると質問者の理解の助けになります。 また、読む側のことを考えた、分かりやすい文章を心がけましょう。
バッドをするには、ログインかつ
こちらの条件を満たす必要があります。
退会済みユーザー
2017/10/29 13:22