質問をすることでしか得られない、回答やアドバイスがある。

15分調べてもわからないことは、質問しよう!

ただいまの
回答率

91.03%

  • JavaScript

    13808questions

    JavaScriptは、プログラミング言語のひとつです。ネットスケープコミュニケーションズで開発されました。 開発当初はLiveScriptと呼ばれていましたが、業務提携していたサン・マイクロシステムズが開発したJavaが脚光を浴びていたことから、JavaScriptと改名されました。 動きのあるWebページを作ることを目的に開発されたもので、主要なWebブラウザのほとんどに搭載されています。

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

解決済

回答 3

投稿

  • 評価
  • クリップ 0
  • VIEW 198

nabettu

score 1

実現したい関数

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

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);;
    }

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

  • 気になる質問をクリップする

    クリップした質問は、後からいつでもマイページで確認できます。

    またクリップした質問に回答があった際、通知やメールを受け取ることができます。

    クリップを取り消します

  • 良い質問の評価を上げる

    以下のような質問は評価を上げましょう

    • 質問内容が明確
    • 自分も答えを知りたい
    • 質問者以外のユーザにも役立つ

    評価が高い質問は、TOPページの「注目」タブのフィードに表示されやすくなります。

    質問の評価を上げたことを取り消します

  • 評価を下げられる数の上限に達しました

    評価を下げることができません

    • 1日5回まで評価を下げられます
    • 1日に1ユーザに対して2回まで評価を下げられます

    質問の評価を下げる

    teratailでは下記のような質問を「具体的に困っていることがない質問」、「サイトポリシーに違反する質問」と定義し、推奨していません。

    • プログラミングに関係のない質問
    • やってほしいことだけを記載した丸投げの質問
    • 問題・課題が含まれていない質問
    • 意図的に内容が抹消された質問
    • 広告と受け取られるような投稿

    評価が下がると、TOPページの「アクティブ」「注目」タブのフィードに表示されにくくなります。

    質問の評価を下げたことを取り消します

    この機能は開放されていません

    評価を下げる条件を満たしてません

    評価を下げる理由を選択してください

    詳細な説明はこちら

    上記に当てはまらず、質問内容が明確になっていない質問には「情報の追加・修正依頼」機能からコメントをしてください。

    質問の評価を下げる機能の利用条件

    この機能を利用するためには、以下の事項を行う必要があります。

回答 3

+3

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

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;
  }
}

投稿

  • 回答の評価を上げる

    以下のような回答は評価を上げましょう

    • 正しい回答
    • わかりやすい回答
    • ためになる回答

    評価が高い回答ほどページの上位に表示されます。

  • 回答の評価を下げる

    下記のような回答は推奨されていません。

    • 間違っている回答
    • 質問の回答になっていない投稿
    • スパムや攻撃的な表現を用いた投稿

    評価を下げる際はその理由を明確に伝え、適切な回答に修正してもらいましょう。

  • 2017/10/29 00:26

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

    キャンセル

  • 2017/10/29 15:21 編集

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

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

    キャンセル

  • 2017/10/29 15:25

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

    キャンセル

  • 2017/10/29 16:50 編集

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

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

    キャンセル

  • 2017/10/29 17:22

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

    キャンセル

+3

 構造化プログラミング(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 の処理を読んでみましょう。

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 さん

投稿

編集

  • 回答の評価を上げる

    以下のような回答は評価を上げましょう

    • 正しい回答
    • わかりやすい回答
    • ためになる回答

    評価が高い回答ほどページの上位に表示されます。

  • 回答の評価を下げる

    下記のような回答は推奨されていません。

    • 間違っている回答
    • 質問の回答になっていない投稿
    • スパムや攻撃的な表現を用いた投稿

    評価を下げる際はその理由を明確に伝え、適切な回答に修正してもらいましょう。

  • 2017/10/29 22:22

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

    キャンセル

checkベストアンサー

+2

別の視点から。

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

という考えがあります。ただし、汎用化すると言っても今必要な機能だけを実装していかないと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行のコードでここまですべきかどうかは時と場合によると思いますが、保守性を重視し、それぞれについて単体テストも用意するのであれば、間違った選択では無いと思います。

投稿

  • 回答の評価を上げる

    以下のような回答は評価を上げましょう

    • 正しい回答
    • わかりやすい回答
    • ためになる回答

    評価が高い回答ほどページの上位に表示されます。

  • 回答の評価を下げる

    下記のような回答は推奨されていません。

    • 間違っている回答
    • 質問の回答になっていない投稿
    • スパムや攻撃的な表現を用いた投稿

    評価を下げる際はその理由を明確に伝え、適切な回答に修正してもらいましょう。

  • 2017/10/29 22:25

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

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

    キャンセル

15分調べてもわからないことは、teratailで質問しよう!

  • ただいまの回答率 91.03%
  • 質問をまとめることで、思考を整理して素早く解決
  • テンプレート機能で、簡単に質問をまとめられる

関連した質問

同じタグがついた質問を見る

  • JavaScript

    13808questions

    JavaScriptは、プログラミング言語のひとつです。ネットスケープコミュニケーションズで開発されました。 開発当初はLiveScriptと呼ばれていましたが、業務提携していたサン・マイクロシステムズが開発したJavaが脚光を浴びていたことから、JavaScriptと改名されました。 動きのあるWebページを作ることを目的に開発されたもので、主要なWebブラウザのほとんどに搭載されています。