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

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

新規登録して質問してみよう
ただいま回答率
85.51%
ESLint

ESLintは、JavaScriptのための構文チェックツール。全検証ルールを自由に on/offでき、独自のプロジェクトに合わせたカスタムルールを容易に設定することが可能。公開されている様々なプラグインを組み込んで使用することもできます。

JavaScript

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

Q&A

解決済

5回答

1024閲覧

【JavaScript】no-extend-native に対処したい【ESLint】(質問タイトル修正)

himejiy3

総合スコア77

ESLint

ESLintは、JavaScriptのための構文チェックツール。全検証ルールを自由に on/offでき、独自のプロジェクトに合わせたカスタムルールを容易に設定することが可能。公開されている様々なプラグインを組み込んで使用することもできます。

JavaScript

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

0グッド

0クリップ

投稿2017/11/03 08:56

編集2017/11/03 23:51

(2017-11-04 8:50頃 追記)
旧タイトル「プロトタイプをよく分かっていなかったようです・・」を変更しました。
当初、「自分が上手くコードを書き替えられないのはプロトタイプの仕組みを理解していないからだ」と考え旧タイトルにしていたのですが、問題は別のところにあったようなので、ベストアンサーを済ませた後ではありますが、タイトルを修正させて頂くことにしました。
何卒、ご了承ください。


以下はカードゲームのシャッフル部分を抜粋したコードで、一応は意図した通りに動いたのですが

javascript

1Array.prototype.shuffle = function() { 2 let i = this.length; 3 while (i > 0) { 4 const j = parseInt(Math.random() * i, 10); 5 i--; 6 [this[i], this[j]] = [this[j], this[i]]; 7 } 8 return this; 9}; 10 11const cards = []; 12for (let i = 1; i <= 8; i++) { 13 cards.push(i); 14 cards.push(i); 15} 16cards.shuffle(); 17 18console.log(cards); // [5, 3, 1, 8, 4, 6, 6, 7, 2, 1, 3, 4, 2, 5, 7, 8] 19

これですとESLintに
Array prototype is read only, properties should not be added. (no-extend-native)
(配列プロトタイプは読み取り専用であり、プロパティを追加しないでください。)
と怒られてしまいました。
で、初心者の私としてはここらへんの標準は分かりませんが、取りあえず
「なるほど、これは好ましい手法ではないのかもしれない」
と思いまして、
怒られない書き方に修正しようとしました。
しかし上手くいかず、ハマってしまいました。

javascript

1const MyArray = []; 2MyArray.prototype.shuffle = function() { 3

とすると
Uncaught TypeError: Cannot set property 'shuffle' of undefined
と言われ、
そりゃそうだと思いながらいろいろ試すのですが上手く動いてくれず、
結局
こんな書き方↓に落ち着いたわけですが

javascript

1const cards = []; 2 3cards.shuffle = function() { 4 let i = this.length; 5 while (i > 0) { 6 const j = parseInt(Math.random() * i, 10); 7 i--; 8 [this[i], this[j]] = [this[j], this[i]]; 9 } 10 return this; 11}; 12 13for (let i = 1; i <= 8; i++) { 14 cards.push(i); 15 cards.push(i); 16} 17cards.shuffle(); 18 19console.log(cards); 20// [4, 8, 6, 7, 1, 5, 1, 7, 6, 4, 2, 8, 3, 5, 2, 3, shuffle: ƒ] 21

確かにこれなら思惑通りに動くのですけど、
これでは、将来的にコードの再利用も考慮したつもりであった、当初の思想から思いっきり外れてしまい、不本意な書き方になってしまいました。
(まぁ、理解不足・知識不足が判明したため、再利用も何もあったものではないのですが。)
何か大きな勘違いがあるかもしれませんし。

ESLintの「no-extend-native」をoffにしても良いのですけど、何だかモヤモヤします。

そこで質問です。
今回の場合ですと、「配列名.shuffle();」と書けば配列の中身がシャッフルされる汎用コードを残しておきたいです。
勉強不足で理解が追い付かないかもしれませんが、当面の模範解答をご教授願えますでしょうか。
よろしくお願いします。

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

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

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

バッドをするには、ログインかつ

こちらの条件を満たす必要があります。

defghi1977

2017/11/03 10:23

プロトタイプを理解しているからこそArray.shuffleを実装できたのであって, Arrayを拡張するのが目的であればESLint側をoffにするのは別におかしくないんじゃないかな?
himejiy3

2017/11/03 11:02

そういうものなのでしょうか。でも「no-extend-native」を回避するコードを書けないままなのは残念です。最後のコードは回避したように感じられませんでしたし。Arrayにくっつける以外のオーソドックスな手法も知っておきたいと思っています。
defghi1977

2017/11/03 11:15

であれば質問の内容と題名が合っていません. 「Array.prototypeに関数を登録せずに配列名.関数名()と記述できるようにするには」とすべきです. 少なくともあなたはプロトタイプを理解しているように見えます.
himejiy3

2017/11/03 11:25

んー、そうですか。正直、Array.prototype~の部分は「書いてみたらなんか出来た」といった程度で、理解して書いた感覚があまり無いんですよね…。その後の修正を試みている間も、理由が分からないエラーも多くて。何だかスポンジの上を歩いている気分です。ふわふわした知識で進んでいる感じで。
guest

回答5

0

既存クラス(JavaScriptの場合はコンストラクタ)に後から変更を加える行為は他の言語でモンキーパッチ等と言われています(モンキーパッチという言葉自体は言語のコミュニティーによって少し異なるので注意が必要です)。このモンキーパッチは一時的な対応やテスト等において有効な手段の一つではありますが、際限なく使うことは危険とされており、特に組み込みオブジェクトに対して行うことは、余程の理由が無い限り使うべきでは無いとされています。(絶対に使うなと言うのでは無く、Polyfillの実現などの妥当な理由があれば、問題ありません)

###なぜ、やってはいけないのか?

まず、ESLintでエラーがでるというとよりも、なぜESLintではデフォルトでエラーとしているのかを理解すべきです。なぜ悪いかがわからないとどう直すべきかもわかりませんので。

色々あって、ファイルを分割することにしました。

main.mjs

JavaScript

1import cards from './cards'; 2import './shuffle'; 3cards.shuffle(); 4 5console.log(cards);

shuffle.mjs

JavaScript

1Array.prototype.shuffle = function() { 2 let i = this.length; 3 while (i > 0) { 4 const j = parseInt(Math.random() * i, 10); 5 i--; 6 [this[i], this[j]] = [this[j], this[i]]; 7 } 8 return this; 9};

cards.mjs

JavaScript

1const cards = []; 2for (let i = 1; i <= 8; i++) { 3 cards.push(i); 4 cards.push(i); 5} 6Array.prototype.shuffle = function() {}; 7cards.shuffle(); 8 9export default cards;

node.jsでnode --experimental-modules main.mjsで実行すれば、同じように動くことを確認できるでしょう。なお、cards.mjsでshuffleを定義しているのは、モジュールでの分割を頼んだテラ君(仮名)によると、どうやらそのあとのcards.shuffle();がエラーになったので、エラーにならないようにダミーで追加したそうです。テラ君曰く、モジュールはそれぞれ独立した環境だから、ダミーが置いてあっても問題ないとのことです。

さらにプロジェクトが進んで、fromが無いimport文は最初に書くと決まったので、main.mjsを次のように書き直しました。

main.mjs (修正版)

JavaScript

1import './shuffle'; 2import cards from './cards'; 3cards.shuffle(); 4 5console.log(cards);

動くは動きますが、まったくシャッフルされなくなってしまいました。一体何が問題なのでしょうか?

テラ君はモジュールで副作用があるのはfromが無いimport文だけだと勘違いしていたようですが、実際はどのような呼び出し方であれ、副作用はあり得ます。今回のようにグローバルオブジェクトであるArrayの一部を変更するような場合、全ての環境でのArrayに影響を及ぼします。shuffle.mjsとcards.mjsの両方で同じ所(Array.prototype.shuffle)を変更しようとすると、後からの変更が前の変更を上書きしますので、import文が入れ替わっただけで、動作が変わるという現象が起きてしまいました。

このように複数のファイルで構成するようになったときに問題が生じます。上の話は短いコードなのですぐにわかりますが、もっと長いコード、より複雑な入れ子になった呼び出しがある場合、すぐに見つけることはできず、予期せぬ不具合として紛れ込んでしまう可能性があります。これを防ぐには、理由なき安易な組み込みへの拡張は行わない、という規約を設ける以外に方法はありません。

###どう修正すればいいのか?

三つほど書きましたが、他にもあると思います。

####エラーを無視する。

組み込みオブジェクトへの変更はバグが起きやすいと言うだけで、バグが必ず起きるという物ではありません。使ってはいけないのでは無く、使うことにリスクがあるということです。そのリスクよりもメリットの方が大きいというのであれば、エラーは無視してそのまま使うという選択も十分あり得ます。

####関数にする。

対象となる配列を引数とする単なる関数にします。何が何でもオブジェクト指向っぽくしたいというのが無ければこれが良いでしょう。

JavaScript

1function shuffle(list) { 2 let i = list.length; 3 while (i > 0) { 4 const j = parseInt(Math.random() * i, 10); 5 i--; 6 [list[i], list[j]] = [list[j], list[i]]; 7 } 8 return list; 9}; 10 11const cards = []; 12for (let i = 1; i <= 8; i++) { 13 cards.push(i); 14 cards.push(i); 15} 16shuffle(cards); 17 18console.log(cards);

####Cardsクラス(コンストラクタ)を作り、そこに実装する。

オブジェクト指向っぽくしたいのであればクラスを作ってしまいます。

JavaScript

1class Cards { 2 constructor() { 3 this.list = []; 4 for (let i = 1; i <= 8; i++) { 5 this.list.push(i); 6 this.list.push(i); 7 } 8 } 9 shuffle() { 10 let i = this.list.length; 11 while (i > 0) { 12 const j = parseInt(Math.random() * i, 10); 13 i--; 14 [this.list[i], this.list[j]] = [this.list[j], this.list[i]]; 15 } 16 return this.list; 17 } 18} 19 20const cards = new Cards; 21cards.shuffle(); 22 23console.log(cards);

ESLintは危ないコードを書いていませんかというただのお知らせです。**それに必ず従わなければならないというものではありません。**しかし、なぜ、エラーや警告になるのかを理解せずに無視するのもいけません。エラーなどが出ないことだけを目的にすると、ちぐはぐな対応になり、せっかくと指摘が無駄になってしまいます。「とりあえず出ないようにしよう」とはしないでください。まずは、なぜ駄目なのかということを調べて、理解する所から始めてください。何が問題なのかが調べてもわからなければ、teratailで聞けば良いだけなんですから。きっと皆さんが親切に答えてくれますよ。

投稿2017/11/03 23:03

raccy

総合スコア21733

バッドをするには、ログインかつ

こちらの条件を満たす必要があります。

himejiy3

2017/11/03 23:35

回答ありがとうございます。 既にタッチの差でベストアンサーで閉じてしまっていたことをご容赦ください。 『ESLintでエラーがでるというとよりも、なぜESLintではデフォルトでエラーとしているのかを理解すべきです。』 そう、そこです。この警告はどこまで従うべきなのか、その判断が、手持ちの書籍と自分なりのネット検索ではできませんでした。 完全な個人の趣味といえども、独自の道を歩むつもりは毛頭無く、長いものに巻かれる書き方をしたいので。 『なぜ駄目なのかということを調べて、理解する所から始めてください。』 はい、今回の件は「このエラーの詳細を知りたい」という質問でいったん留めておくべきでした。 結論を急ぎ過ぎて、漠然とした問いになってしまったことを申し訳なく思います。
guest

0

ベストアンサー

正直なところ, どう汎用的に作りたいかが見えないため答えようがなく, 再利用だけが目的であれば「どうとでもなる」というのが個人的な意見です.

ただ, 開発の規模によって扱いやすい形が変化するので, その傾向は掴んでおくと良いでしょう. (Array.prototypeの拡張は大規模開発において名前の競合を引き起こすリスクが高まるため, Lintで明示的に禁止できる等)


ここではArrayオブジェクトを継承する方法を示します.

以下はプロトタイプチェーンでArrayオブジェクトを継承したCardsオブジェクトを生成しています.

JavaScript

1function Cards() { 2 this.push(...arguments); 3} 4Cards.prototype = new Array(); 5Cards.prototype.shuffle = function () { 6 let i = this.length; 7 while (i > 0) { 8 const j = parseInt(Math.random() * i, 10); 9 i--; 10 [this[i], this[j]] = [this[j], this[i]]; 11 } 12 return this; 13}; 14let cards = new Cards(); 15for (let i = 1; i <= 8; i++) { 16 cards.push(i); 17 cards.push(i); 18} 19cards.shuffle(); 20console.log(cards);

ECMAScript2015が利用できる環境であれば次のようなClass構文を使ってArrayオブジェクトを継承することも可能です.

JavaScript

1class Cards extends Array { 2 constructor() { 3 super(...arguments); 4 } 5 shuffle() { 6 let i = this.length; 7 while (i > 0) { 8 const j = parseInt(Math.random() * i, 10); 9 i--; 10 [this[i], this[j]] = [this[j], this[i]]; 11 } 12 return this; 13 } 14} 15let cards = new Cards(); 16for (let i = 1; i <= 8; i++) { 17 cards.push(i); 18 cards.push(i); 19} 20cards.shuffle(); 21console.log(cards);

後はこのCardsオブジェクトを使い回せば良いのです.

投稿2017/11/03 12:25

defghi1977

総合スコア4756

バッドをするには、ログインかつ

こちらの条件を満たす必要があります。

himejiy3

2017/11/03 13:23

すみません、我ながら「落としどころの見えない質問になってしまっているなぁ」と思います。 確かに再利用だけが目的なら、自力解決が出来なくもなく、自分でも理解できている関数化による使い回しで済ませれば事足りました。 回答感謝です。 1番目は自分なりに試行錯誤していた時、かなり近付いていましたが、 「this.push(...arguments);」と書く発想に到達できませんでした・・。 2番目のclass命令による構文は、手持ちの参考書には記述があったものの、頭にはまるで入っていませんでした。他言語の経験者は理解しやすいそうですが・・。今一度、本を読み返しておくことにします。こうして回答コードを見ると、この解決策は面白いと感じました。
himejiy3

2017/11/03 22:58

飽くまで「今回のプログラムでは」という条件付きではありますが、 こちらの回答コードの利用で、当初の自分が「こうしたかった」という形に出来ました。 でも次回は、単純な関数化でまとめようと思います。 おかげで勉強になりましたけど、そこまでこだわる部分でなかったのは明白です。 趣味で楽しむプログラミングならではの足踏みでした。 ちなみに、こちらの回答2番目のclass命令での対処法は、上級者ですと constructor() { super(...arguments); } の部分は省略するところのようですが、わざわざ書いてくださっていたことで 参考書の記述に辿り着きやすかったです。 皆さん、ありがとうございました。 非常に、大変迷いましたが、今回はこちらの回答をベストアンサーとさせていただきます。
guest

0

今回の場合ですと、「配列名.shuffle();」と書けば配列の中身がシャッフルされる汎用コードを残しておきたいです。

ESLintの「no-extend-native」

は、仕組み上両立できません(何か技巧的なことをすれば回避できなくはないかもしれませんが、それはno-extend-nativeをごまかしているのに過ぎず、本質的にはどうしようもありません)。

標準で使われているプロトタイプにメソッドを追加するのは「プロトタイプ汚染」と言われるように、忌避されることも多い手法です。再利用を考えれば、shuffle(配列)という形の単なる関数としておいたほうがいいと思います。

投稿2017/11/03 11:01

maisumakun

総合スコア145062

バッドをするには、ログインかつ

こちらの条件を満たす必要があります。

himejiy3

2017/11/03 11:15

ほうほうほうほう、そうなのですか。そういう部分も知りたかったのです。自分は何が分かっていないのかも分からない段階なので。 単なる関数で書いて再利用に備えるのは出来ると思います。それで良いとされるなら気が楽になるところです。
maisumakun

2017/11/03 11:27

require/exportsやimport/exportを使う現代のJavaScript環境では、ライブラリから供給されるものも、基本的に変数でやり取りされます。 「prototypeの書き換え」は、「新しい標準で登場したメソッドをJavaScriptコードで補う」時以外は、ほぼ使われません。
himejiy3

2017/11/03 12:05

回答に加えて「変数でやり取り」「prototype書き換えはほぼ使われない」といったお話で、本当に知りたかった部分に近付いた感じがします。 今回のつまづきは正解が見えてこなかったもので、ESLintの警告に従うべきか、offで対処すべきか、はたまた、踏ん張ってプロトタイプの知識を深めるべきか、関数でサッと済ませて次に進むべきか、判断できず、取りあえず足踏みしてしまっていました。 方向性が見えてきたように思います。
guest

0

//名前の衝突防止 const shuffle = Symbol('shuffle'); //shuffleの実装と仮定 _shuffle(){ return [] }; Object.defineProperty(Array.prototype, shuffle, { value: _shuffle, writable: true, configurable: true, enumerable: false }); const numbers = [1, 2, 3, 4, 5]; console.log(numbers[shuffle]()); // []

numbers.shuffle()ではなく、numbers[shuffle]()になるのがイケテナイ。

投稿2017/11/03 14:24

編集2017/11/03 23:46
HayatoKamono

総合スコア2415

バッドをするには、ログインかつ

こちらの条件を満たす必要があります。

himejiy3

2017/11/03 21:47

これはなるほどと思いました。 先のdefghi1977さんの回答でも「(...arguments)」とされていて、「あっ」と思った次第です。 自分が迷走していた当時、その点には全く考えが及んでいませんでした。
guest

0

基本的な配列に対して自分で定義したメソッドをポン付けするのはラッパーオブジェクトを使えば実現できると思います。
でもけっこう面倒なので、別のやり方を提案します。
シャッフルする配列をプロパティに持ったクラス(jsなのでfunctionだけど)を用意するのはどうでしょうか。

javascript

1function Cards(){ 2 let numbers = []; 3}; 4 5Cards.prototype.set = function(c){ 6 this.numbers = c; 7} 8 9Cards.prototype.shuffle = function(){ 10 let arr = this.numbers; 11 for(let i = arr.length - 1; i > 0; i--){ 12 const r = Math.floor(Math.random() * (i + 1)); 13 const tmp = arr[i]; 14 arr[i] = arr[r]; 15 arr[r] = tmp; 16 } 17 this.numbers = arr; 18} 19 20var case1 = new Cards(); 21case1.set([1,2,3,4,5,6,7,8,1,2,3,4,5,6,7,8]); 22case1.shuffle(); 23console.log('case1.numbers', case1.numbers);

でもここまで書いて気づいたんですが、そもそもprototype使う必要はないですよね。
普通に配列をシャッフルする関数を書けばいいだけ。

javascript

1function shuffle(arr){ 2 for(let i = arr.length - 1; i > 0; i--){ 3 const r = Math.floor(Math.random() * (i + 1)); 4 const tmp = arr[i]; 5 arr[i] = arr[r]; 6 arr[r] = tmp; 7 } 8 return arr; 9}; 10 11var cards = [1,2,3,4,5,6,7,8,1,2,3,4,5,6,7,8]; 12cards = shuffle(cards);

結論:シャッフル処理を外出ししたいなら関数化すればok。

投稿2017/11/03 11:14

ooeok

総合スコア469

バッドをするには、ログインかつ

こちらの条件を満たす必要があります。

himejiy3

2017/11/03 11:41

「prototypeを使う必要はない」・・はい、こだわりすぎましたかね・・。 「関数化すればok。」・・結局、そこに行き着くものでしょうか。世の先輩方もそういう風に組んでいるもんだ、となれば安心感はあります。 自分的に、ちょっと背伸びしすぎていた部分もあったかもしれません。
guest

あなたの回答

tips

太字

斜体

打ち消し線

見出し

引用テキストの挿入

コードの挿入

リンクの挿入

リストの挿入

番号リストの挿入

表の挿入

水平線の挿入

プレビュー

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

ただいまの回答率
85.51%

質問をまとめることで
思考を整理して素早く解決

テンプレート機能で
簡単に質問をまとめる

質問する

関連した質問