入力された文字列の行数を返すJavascriptの関数

Question

###実現したい関数
入力された文字列に対して、その行数を返す関数を作りたいのですが、
その際最小値と最大値を設定する必要があります。

function(text) {
      const minHeight = 5;
      const maxHeight = 10;
      const textHeight = text.match(/\n/g) ? text.match(/\n/g).length + 1 : 0;
      return Math.max(Math.min(textHeight, maxHeight), minHeight);;
    }

書いてみたのですが、もうちょっとわかりやすく書けないかな〜と思っています。
何かもっといい書き方ありませんか？
コードの短さよりもわかりやすさを優先とします。

Answer 1

構造化プログラミング(structured programming)

構造化プログラミング(structured programming)と呼ばれる思想があり、その原則に「関数は単一の入口と単一の出口を持つべきである」というものがあります。

'use strict';
function getHeightClamped (text) {  // 仮引数 (単一の入口)
  const minHeight = 5;
  const maxHeight = 10;
  const textHeight = text.split("\n").length; // 返り値となる変数を作る

  if (textHeight < minHeight) {
    textHeight = minHeight; // 返り値に最小値を代入する
  } else if (textHeight > maxHeight) {
    textHeight = maxHeight; // 返り値に最大値を代入する
  }

  return textHeight;  // 返り値を返す (return 節を一度だけ使う事で単一の出口となる)
}

この原則の利点として、アルゴリズムを書きだした時にすっきりした構造となることが挙げられます。
入口/出口が複数あると、考えるパターンが複雑化してしまい、人間の目で見て追いきれない事があります。

しかしながら、構造化プログラミングには入口/出口が狭い都合上、コードの階層が深くなってしまう欠点があり、最近の考え方としては「例外処理は値を先に返す事で追い出してしまい、後続コードを正常系処理のみにする事ですっきりしたアルゴリズムにしよう」が主流なようです。
実際にJavaScriptの基本仕様である ECMAScript でも同様のアルゴリズムが組まれています。
一例として、ECMAScript 5.1 規定の Array.isArray の処理を読んでみましょう。

• 15.4.3.2 Array.isArray ( arg ) - ECMA-262 Edition 5.1

1. If Type(arg) is not Object, return false.
2. If the value of the [[Class]] internal property of arg is "Array", then return true.
3. Return false.

1. 引数 arg が Object 型でない場合は false を返す
2. 引数 arg の内部プロパティ [[Class]] が "Array" ならば、true を返す
3. false を返す

1., 2., 3. のstepで**「そうでなければ(else)」の文言がないのは、前stepで値を返す事で例外処理が終わっている為**です。
互換コード(Polyfillコード)は次のように書けます。

Object.defineProperty(Array, 'isArray', {writable: true, enumerable: false, configurable: true,
  value: function isArray (arg) { // 1. If Type(arg) is not Object, return false.

    if (Object(arg) !== arg) {
      return false;
    }

    if (Object.prototype.toString.call(arg) === '[object Array]') { // 2. If the value of the [[Class]] internal property of arg is "Array", then return true.

      return true;
    }

    return false;  // 3. Return false.
  }
});

このコードは「入口が一つ、出口は複数」ですが、正常系処理に限っては「入口が一つ、出口が一つ」とする事で構造化プログラミングの理念の則ってすっきりしたコードを書けます。

余談ですが、入口を一つにする原則は、「参照透過性が成り立つコード」を組む時にも通用する考え方となります。
参照透過性が成り立たないコードは「引数とは別に入口がある」と考えられるからです。

複数の出口を持つコード

私は基本的に汎用性重視でコードを組みます。

• 最小値/最大値は引数で指定します(オプション)
• 引数無しは無制限です

'use strict';
function getHeightClamped (string /* [minHeight [, maxHeight] */) {
  const argumentsLength = arguments.length,
        minHeight = argumentsLength > 1 ? Number(arguments[1]) : 1,
        maxHeight = argumentsLength > 2 ? Number(arguments[2]) : 1/0,
        stringHeight = string.split(/\r\n|[\n\r]/).length;

  if (stringHeight < minHeight) {
    return minHeight;
  }

  if (stringHeight > maxHeight) {
    return maxHeight;
  }

  return stringHeight;
}

console.log(getHeightClamped('a\r\nb\nc\rd\ne')); // 5 (\r\n, \r, \n の3タイプの改行コードを全て認識する、第二引数以降省略したら無制限)
console.log(getHeightClamped('a\nb\nc\nd\ne', 1, 3)); // 3 (最小値: 1 / 最大値: 3)
console.log(getHeightClamped('a\nb', 3)); // 3 (最小値: 2 / 最大値: 無制限)

return 節が複数存在する事から、このコードは「構造化プログラミング」の原則に反しています。
このコードは「例外処理系」と「正常処理系」を分割する事に利点があると考える為、else-if 節で繋がず、空行を開ける事で「それぞれの例外処理」を細かく分けるように書いています。

更新履歴

• 2017/10/29 16:37 「構造化プログラミング」の節を追記。それに伴い、「複数の出口を持つコード」の節に補足説明を追記。

Re: nabettu さん

Comments

[退会済みユーザー]

think49さん
回答ありがとうございます。
「構造化プログラミング」の原則ですね、たしかにそのほうが透過性は高まりますね。
しかしガード節を取り入れたほうが汎用性高まるというのも納得なので、この例も非常にわかりやすいです。ありがとうございます。

Answer 2

別にそれでも良いと思いますが、より愚直に計算するとこうでしょうか。

function getHeightClamped(text) {
  const minHeight = 5;
  const maxHeight = 10;
  const textHeight = text.split("\n").length;
  if (textHeight < minHeight) {
  	return minHeight;
  } else if (textHeight > maxHeight) {
  	return maxHeight;
  } else {
  	return textHeight;
  }
}

Comments

[退会済みユーザー]

splitつかったほうがスマートですね！
というのもmatchでは空文字列だった場合にnullになってしまうので三項演算子を挟んでいて、私のコードはそれが見づらくなる原因の一つだったので。
参考になります！

[think49]

答えは出ていると思いますが、質問をクローズしないのは「まだ納得できない何か」があるからでしょうか。
私から見て、karamarimo さんのコードには問題と思える点はほとんどありません。
あえて、指摘するのであれば、
- if 文と const の間に空行を入れる
- else-if を if に変更して空行を入れる

「わかりやすさ」は主観によるところが大きいので、nabettu さんの中で「わかりにくいと思っているポイント」を具体化してみると、他の方からもより前向きな提案が出てくると思います。

[karamarimo]

「else-if を if に変更して空行を入れる」のはなぜでしょうか？

[think49]

To: karamarimo さん
言い訳がましくなってしまいますが、個人的にはこの質問は「karamarimo さんの回答をベストアンサーに選んでクローズされるだろう」と思っていたので、深く関わるつもりはありませんでした。
なかなかクローズされないので、気になってコメントしましたが、他の人に強く主張する程の理由でもないと考えています。
思想的な理由が大きいので、karamarimo さんを否定する意図はないです(インデントのtab/space論争みたいなものです)。
「質問者さんが考えるきっかけになれば」と思い、あえて具体的なポイントを書いてみました。

> 「else-if を if に変更して空行を入れる」のはなぜでしょうか？
私が考える理由は、
- return で処理が終了している事から else が不要な為
- 空行を入れる事でそれぞれの処理ブロックを明確にする為
- 不必要に階層を深くしない為(else 節の部分は一段深くなっています)
どちらかといえば、else-if は構造化プログラミングと相性が良いと考えていて、それについては長くなるので回答欄に追記しました。

[karamarimo]

丁寧にお答え頂きありがとうございます。
なるほど、コーディングスタイルに関しては疎いもので、いろいろ考えるべきことがあるんですね。

Answer 3

別の視点から。

• 関数は細かく分ける。一つの関数に一つのロジックしか書かない。
• 関数に汎用性を持たせる。汎用的な関数から特殊化していく。

という考えがあります。ただし、汎用化すると言っても今必要な機能だけを実装していかないとYAGNIに反して無駄な処理が多くなりますので、注意が必要です。

// 範囲内に無い場合はその範囲に収める
function correctRange(value, { min = -Infinity, max = Infinity }) {
  if (min > max) {
    throw new Error(`min is over than max: min ${min}, max: ${max}`);
  }
  if (value < min) {
    return min;
  }
  if (value > max) {
    return max;
  }
  return value;
}

// 文字列中の文字の数を数える
function countChar(str, c) {
  return Array.from(str).filter(s => s === c).length;
}

// 文字列の行数を数える
function countRow(str) {
  const eolCount = countChar(str, '\n');
  const eolTemrinator = str.length === 0 || str.endsWith('\n');
  const lastLineCount = eolTemrinator ? 0 : 1;
  return eolCount + lastLineCount;
}

// 文字列中の文字の数を数え、範囲内に収める
function countRowCorrectedRange(str, range) {
  return correctRange(countRow(str), range);
}

// 文字列中の文字の数を数え、5から10に収める
function countRowFrom5To10(str) {
  return countRowCorrectedRange(str, { min: 5, max: 10 });
}

上は一例に過ぎません。これより良い分け方があるかも知れません。このように書いた場合の利点は次のようなことです。

• 再利用しやすい。例えば、5から10ではなく20から50が必要になっても、countRowFrom5To10を1行変えた(といっても、元々1行だけですが)だけの関数を用意すれば良い事になります。
• 変更しやすい。例えば、最後が改行で終わらない文字列の場合にその行も1行も数えるかどうかが問題になるとき、countRowのロジックだけに集中して修正することができます。
• 改良しやすい。例えば、文字数をカウントするcountCharのパフォーマンスが良くない(実際に上のコードはあまり良くないと思います)として、この高速化を考えたとき、countCharの中身だけに集中して改良することができます。

たかが4行のコードでここまですべきかどうかは時と場合によると思いますが、保守性を重視し、それぞれについて単体テストも用意するのであれば、間違った選択では無いと思います。

Comments

[退会済みユーザー]

raccyさん
回答ありがとうございます。

コストをかけて汎用性・保守性をどこまで高めるか、は確かに悩みどころがありますね。
しかしテストまで考慮するとこの形まで持っていくほうが追々負債が残らなそうですので、ベストアンサーとさせていただきました。