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

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

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

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

Q&A

解決済

6回答

345閲覧

for文での上手な短縮の仕方がわかりません。

hectopascal1013

総合スコア466

JavaScript

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

0グッド

0クリップ

投稿2018/09/23 09:40

前提・実現したいこと

javascriptで乱数Pk(xk,yk)*3点(k=0~2)を発生させ三角形とし、
ベクトル計算から重心を求めるものを想定しています。(拡張して三角形のその他5心も求めるものも別途書きました。そちらについては、今後再質問させていただくかもしれません。)

プログラムはできているのですが、素人目にも増長になりすぎて、短縮している過程ですが、
うまい方法が思いつきません。

具体的には該当ソースコードの以下の部分の省略に手間取っております。
var x0=parseFloat(document.getElementById('x0').value);
var x1=parseFloat(document.getElementById('x1').value);
var x2=parseFloat(document.getElementById('x2').value);
var y0=parseFloat(document.getElementById('y0').value);
var y1=parseFloat(document.getElementById('y1').value);
var y2=parseFloat(document.getElementById('y2').value);
この部分を短くfor文を使って回したいです。

発生している問題・エラーメッセージ

該当のソースコード

javascript

1<!DOCTYPE html> 2<html lang="ja"> 3<head> 4<meta charset="utf-8"> 5<style></style> 6</head> 7<body> 8 9<script> 10 function atk(){ 11 12var fm = document.getElementById('fm'); 13fm.innerHTML=""; 14 15var arr=[]; 16 17for(k = 0; k < 3; k++){ 18 var inputX = document.createElement('input'); 19 inputX.type = 'number'; 20 inputX.size = 10; 21 inputX.id = `x${k}`; 22 inputX.placeholder = `x${k}`; 23 24 var inputY = document.createElement('input'); 25 inputY.type = 'number'; 26 inputY.size = 10; 27 inputY.id = `y${k}`; 28 inputY.placeholder = `y${k}`; 29 inputY.value = `y${k}`; 30 31 var br = document.createElement('br'); 32 33 var before = document.createTextNode(`p${k}(`); 34 var comma = document.createTextNode(','); 35 var after = document.createTextNode(')'); 36 37 fm.append(before); 38 fm.append(inputX); 39 fm.append(comma); 40 fm.append(inputY); 41 fm.append(after); 42 fm.append(br); 43 44 var p=(Math.random()*1000); 45 var q=(Math.random()*1000); 46 47 document.getElementById(`x${k}`).value=p; 48 document.getElementById(`y${k}`).value=q; 49 arr.push({x:p,y:q}); 50} 51 52var x0=parseFloat(document.getElementById('x0').value); 53var x1=parseFloat(document.getElementById('x1').value); 54var x2=parseFloat(document.getElementById('x2').value); 55var y0=parseFloat(document.getElementById('y0').value); 56var y1=parseFloat(document.getElementById('y1').value); 57var y2=parseFloat(document.getElementById('y2').value); 58 59 //g 60const g=[]; 61const gx=(x0+x1+x2)/3; 62const gy=(y0+y1+y2)/3; 63g.push(gx); 64g.push(gy); 65alert(g); 66} 67 68</script> 69 70<form id="fm"></form> 71<input type="button" id="Atk" onclick="atk()" value="solve"> 72 73 74</body> 75</html>

試したこと

for(k…){}の中に
x[k]=parseFloat(document.getElementById('y${k}').value);
とした所、
今まで正常に表示されていたp1,p2ポイントのxy座標のinputタグまで表示されなくなってしまいました。
そこで、for(m…)のループをkの後に追加しkをmと置き換えて回してみましたが、
今度はxy座標の取得に失敗してしまいました。
kのループの中ではp,q座標配列取得が最終項しか何故かできず、
今後のその他5心を求めるために必要な情報として配列として残すのに失敗してしまいました。

わかりにくい質問で申し訳ありませんが、何卒よろしくお願い申し上げます。

補足情報(FW/ツールのバージョンなど)

ここにより詳細な情報を記載してください。

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

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

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

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

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

guest

回答6

0

こんにちは。

この回答で説明する考え方、コード追加、修正によって作成したコードを、以下のURL

**動作確認用のサンプル: ** https://jsfiddle.net/jun68ykt/g69zsfha/2/

に上げましたのでお試しください。(※ランダムな座標を発生させる、[generate]ボタンを追加しています)

ご質問に挙げられているコードを、私がリファクタするとしたら、

  • ロジックとビュー(UI)をできるだけ分離する。

ことを目指します。このご質問でロジックとは主に

  • 乱数による座標の生成
  • 重心の計算

の2点で、ビューの役割は主に以下の2点です。

  • 生成された複数の座標を対応する<input> の value 属性に反映させる。
  • ボタンがクリックされたときに、重心計算を行い結果を表示する。

また、

  • ランダムな座標を持つ複数の点を(まとめて)どこに保存しておくか?

という点もコードを見通しよく保つために重要かなと思いますが、これの解決策として、data-属性を使いました。

上記の指針により、以下のようなコード修正、追加を行いました。

1. ロジック

1.1 Pointコンストラクタの追加

xy座標上の点を表す、Pointコンストラクタを追加します。

javascript

1 const Point = function(x, y) { 2 this.x = x; 3 this.y = y; 4 }; 5 6 Point.prototype.toString = function() { 7 return `x: ${this.x}, y: ${this.y}`; 8 };

1.2 ランダムな座標を持つ複数の点の生成

生成する点の数を受け取り、その数のPointオブジェクトを持つ配列を返す関数を作ります。座標はご質問と同じく、Math.random() * 1000 でランダムに生成させます。

javascript

1 // 乱数を使ってランダムな座標を生成 2 function generateRandomPoints(numberOfPoints) { 3 return [...Array(numberOfPoints)].map(_ => 4 new Point(Math.random() * 1000, Math.random() * 1000) 5 ); 6 }

1.3 重心の取得

与えられた Pointオブジェクト(あるいは、Pointと同様にxyをプロパティに持つオブジェクト)の配列から、これらの重心となるPointを返す関数を作ります。

javascript

1 // 重心の取得 2 function getCenterOfGravity(points) { 3 if (!points || !points.length) 4 return null; 5 6 return new Point( 7 points.reduce((sum,p) => sum + p.x, 0) / points.length, 8 points.reduce((sum,p) => sum + p.y, 0) / points.length 9 ); 10 }

2. ビュー

2.1 3点の座標を表示するHTMLは<form>の中に直接書く。

以下のように <form>の中に直接書き、かつ、各 <input>"x"または"y" のclass属性を追加し、次の 2.2 で使用します。

html

1 <form id="fm"> 2 p0( 3 <input type="number" size="10" id="x0" class="x" placeholder="x0">, 4 <input type="number" size="10" id="y0" class="y" placeholder="y0">)<br> 5 p1( 6 <input type="number" size="10" id="x1" class="x" placeholder="x1">, 7 <input type="number" size="10" id="y1" class="y" placeholder="y1">)<br> 8 p2( 9 <input type="number" size="10" id="x2" class="x" placeholder="x2">, 10 <input type="number" size="10" id="y2" class="y" placeholder="y2">)<br> 11 </form>

五角形の重心を求める場合は、上記と同様に p3p4 に相当するHTMLを追加します。

2.2 与えられた複数のPointの座標をHTMLに反映

行うことは以下の2点です。

  • 指定されたPointオブジェクトの配列の各要素の座標を、対応する <input>の value 属性に設定
  • 指定されたPointオブジェクトの配列をJSON化して、<form>の data-points 属性に設定

javascript

1 // 与えられた複数のPointの座標をHTMLに反映 2 function renderPoints(points) { 3 4 // 対応する <input>の valueに設定 5 ['x', 'y'].forEach(function(axis) { 6 const inputs = document.getElementsByClassName(axis); 7 for (let i=0; i < inputs.length && i < points.length; i ++ ) 8 inputs[i].value = points[i][axis]; 9 }); 10 11 // <form> のdata-points属性にpointsをJSONにした文字列を設定 12 const form = document.getElementById('fm'); 13 form.dataset.points = JSON.stringify(points); 14 }

2.3 ランダム座標を生成するトリガーになるボタンとクリックハンドラ設定

HTMLに以下のようなボタンを追加します。

html

1<input type="button" id="Generate" value="generate">

上記のボタンをクリックすると、ランダムな座標が生成され、HTMLに反映されるようにします。

javascript

1const NUMBER_OF_POINTS = 3;

javascript

1 // generateボタンにclickハンドラを設定 2 document.getElementById('Generate').addEventListener('click', 3 function() { 4 const randomPoints = generateRandomPoints(NUMBER_OF_POINTS); 5 renderPoints(randomPoints); 6 } 7 );

2.4 solveボタンにclickハンドラを設定

solveボタンがクリックされたときに重心が計算され、表示されるようにします。重心を計算する対象のPointオブジェクトの配列は、<form>の data-points 属性にJSONで入っている想定なので取り出して、これを重心を取得する関数に渡します。

javascript

1 // solveボタンにclickハンドラを設定 2 document.getElementById('Atk').addEventListener('click', 3 function() { 4 const form = document.getElementById('fm'); 5 if (form.dataset.points) { 6 const points = JSON.parse(form.dataset.points); 7 const g = getCenterOfGravity(points); 8 alert(g); 9 } 10 } 11 );

ご質問に

具体的には該当ソースコードの以下の部分の省略に手間取っております。

var x0=parseFloat(document.getElementById('x0').value);
var x1=parseFloat(document.getElementById('x1').value);
var x2=parseFloat(document.getElementById('x2').value);
var y0=parseFloat(document.getElementById('y0').value);
var y1=parseFloat(document.getElementById('y1').value);
var y2=parseFloat(document.getElementById('y2').value);

とありました。上記の6行では、HTMLに反映された各点の座標を取得していますが、私の回答のコードで、これと同じ働きをしているのは以下の行です。

javascript

1 const form = document.getElementById('fm'); 2 if (form.dataset.points) { 3 const points = JSON.parse(form.dataset.points);

以上、参考になりましたら幸いです。

投稿2018/09/24 03:38

編集2018/09/24 03:51
jun68ykt

総合スコア9058

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

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

hectopascal1013

2018/09/24 06:07

ご指導ありがとうございます。illustratorでのjavascript表記を念頭にしてますので、JSONなどが使えるかは定かではないですが、純粋なjavascriptの勉強としてご指導感謝します。こういった方法は、一人では思いつかないものですので、大変にありがたいです。
hectopascal1013

2018/09/24 09:07

Generateボタンとsolveボタンを分けられた理由ってなんでしょうか?(素朴な疑問です。) あと、質問内容が悪かったようで訂正します。 三角形の5心(重心、内心、外心、垂心、傍心)についてのプログラムですので、 今の所5角形拡張は考えておりません。すいません伝わりづらいようでしたので、訂正します。 配列流用は内心以後の5心でも座標を使えるということでした。 ともあれ、自分で書いたものをできるだけ簡潔にしたいという思いもあり、 練習がてら、皆様のご意見ご指導を仰いでますので、ご指摘いただいた点は視点が広がりますので、何かと助かっております。引き続きよろしくお願いいたします。
jun68ykt

2018/09/24 10:01 編集

> Generateボタンとsolveボタンを分けられた理由ってなんでしょうか?(素朴な疑問です。) ご質問ありがとうございます。私の回答では ・ランダムな座標を持った点の配列を生成すること と ・重心を求めること とを、それぞれ個別の関数で行うようにし、それぞれの関数名を見れば何をする関数かがすぐ分かるようなコードにしましたが、これは「私ならこう書くだろうな」という個人的な考えに過ぎません。 > Generateボタンとsolveボタンを分けられた のは、「ランダムな座標の点の生成」と「重心の計算」とを分離したことを、UIからも明確に視認できるようにするためでした。ですので「Generateボタンとsolveボタンとを分けるべき」と主張したいわけではありませんし、質問者様が 「solveボタンのクリックだけで、3点の生成から重心の計算まですべてが行われることが望ましい」 ということであれば、そのような1個のボタンだけのUIにされましても全く問題はないと思います。
hectopascal1013

2018/09/24 11:15

ご回答ありがとう御座います。 UIを意識しているということですね?わかりました。 一応、私の最初の質問(今回のものではなくて、本当に最初期の質問です。)に立ち返りますと、 Adobe Illustrator上でjavascriptを動かすということを念頭においておりまして、 その点では、UIに対する独自仕様が強いものとなっているようですので、また別途勉強したいと考えております。 今回の作例では、ですから、あまりその辺は意図せず、1Functionでできるだけ済ませられるように考えておりました。 前提の段を飛ばしてしまいましたので、ソコは私のミスです。 ただ、純粋なjavascriptの勉強としては、UIも考えて仕様を分けるというのは、仰る通り理想的で、今までの私の考えにないことでしたので、大変為になりました。 ボタンを分けてもエラーを起こさず利用できるという点では、例えば乱数生成時ではなくて、正三角形時や二等辺三角形時など、特定の値を後から埋め込むタイプなどにも応用が効きそうで、とても役に立つ流れになっています。ありがとうございます。
guest

0

Don't repeat yourself (DRY)

基本的にはDRYの原則に従えば、コードは冗長ではなくなります。

同じコードをリピートしない為には、

  • 同じ演算結果を変数にキャッシュする
  • 引数だけが違う同じ動作は、配列+繰り返し構文の複合を使う
  • 同じコードを関数にまとめる

コードを短縮化

関数と繰り返し構文は二者択一の関係にあります。
下記は、繰り返し構文で解決したコードです。

JavaScript

1<form id="fm"></form> 2<input type="button" id="Atk" onclick="atk(3)" value="solve" /> 3 4<script> 5'use strcit'; 6function atk (length) { 7 const map = new Map([['x', []], ['y', []]]), df = document.createDocumentFragment(); 8 9 for (var i = 0; i < length; i++) { 10 df.appendChild(document.createTextNode('p' + i + '(')); 11 12 for (let [key, array] of map) { 13 const input = document.createElement('input'), name = key + i, value = Math.random() * 1000; 14 15 input.type = 'number'; 16 input.size = 10; 17 input.name = name; 18 input.placeholder = name; 19 input.value = value; 20 21 df.appendChild(input); 22 df.appendChild(document.createTextNode(',')); 23 array.push(value); 24 } 25 26 df.lastChild.data = ')'; 27 df.appendChild(document.createElement('br')); 28 } 29 30 const form = document.getElementById('fm'), results = []; 31 32 form.innerHTML = ''; 33 form.appendChild(df); 34 35 for (let array of map.values()) { 36 let sum = 0; 37 38 for (let value of array) { 39 sum += value; 40 } 41 42 results.push(sum / array.length); 43 } 44 45 alert(JSON.stringify(results)); 46} 47</script>

length を可変に出来るのが、配列,new Mapのメリットです。
拡張性も視野に入れると、コードの幅が広がると思います。


(2018/09/24 10:35追記)
input要素挿入時に合計値算出をする形に改善したコード。

JavaScript

1'use strcit'; 2function atk (length) { 3 const results = new Map([['x', [0, 0]], ['y', [0, 0]]]), df = document.createDocumentFragment(); 4 5 for (var i = 0; i < length; i++) { 6 df.appendChild(document.createTextNode('p' + i + '(')); 7 8 for (let [key, entry] of results) { 9 const input = document.createElement('input'), name = key + i, value = Math.random() * 1000; 10 11 input.type = 'number'; 12 input.size = 10; 13 input.name = name; 14 input.placeholder = name; 15 input.value = value; 16 17 df.appendChild(input); 18 df.appendChild(document.createTextNode(',')); 19 entry[0] += value; 20 ++entry[1]; 21 } 22 23 df.lastChild.data = ')'; 24 df.appendChild(document.createElement('br')); 25 } 26 27 const form = document.getElementById('fm'); 28 29 form.innerHTML = ''; 30 form.appendChild(df); 31 32 alert([...results.values()].map(entry => entry[0] / entry[1])); 33}

クラス化

コード短縮化が命題でしたので、短縮のみの視点で回答しましたが、私が提案したコードはループ回数を減らす為に複数の役割を同一コード上で実行しており、それがコードの可読性を下げている面はあると思います。
パフォーマンス寄りにチューニングすれば、「密結合」が進み、保守性にチューニングすれば、「疎結合」が進みます。
このバランスは人それぞれですが、「座標」の視点で暮らす化したコード例を書いておきます。

基本的な方針は、次の3つの処理を分化する事にあります。

  • データを生成する (new Coordinate, new CoordinateList)
  • データを使って演算する (CoordinateList#getCenter())
  • データを出力する (DOMノードの挿入)

特にDOMノード系は処理を分けた方が良いと思います。後は、論理エラーをクラス内で排除します。

HTML

1<form id="fm"></form> 2<input type="button" id="atk" value="solve" form="fm" /> 3 4<script src="coordinate-list-0.1.0.js"></script> 5<script> 6'use strcit'; 7(function () { 8 function generateDf (coordinateList) { 9 const df = document.createDocumentFragment(); 10 11 for (let i = 0, entries = coordinateList.entries(), length = entries.length; i < length; i++) { 12 df.appendChild(document.createTextNode('p' + i + '(')); 13 14 for (let [key, entry] of entries[i]) { 15 const input = document.createElement('input'), name = key + i, value = Math.random() * 1000; 16 17 input.type = 'number'; 18 input.size = 10; 19 input.name = name; 20 input.placeholder = name; 21 input.value = value; 22 23 df.appendChild(input); 24 df.appendChild(document.createTextNode(',')); 25 } 26 27 df.lastChild.data = ')'; 28 df.appendChild(document.createElement('br')); 29 } 30 31 return df; 32 } 33 34 function createRandomCoordinateList (dimensionList, length) { 35 const coordinateList = []; 36 37 for (let i = 0; i < length; ++i) { 38 const coordinate = []; 39 40 for (let dimension of dimensionList) { 41 coordinate.push([dimension, Math.random() * 1000]); 42 } 43 44 coordinateList.push(coordinate); 45 } 46 47 return new CoordinateList(coordinateList); 48 } 49 50 function handleClick (event) { 51 const coordinateList = createRandomCoordinateList(this.dimensionList, this.length), form = event.target.form; 52 53 form.innerHTML = ''; 54 form.appendChild(generateDf(coordinateList)); 55 56 console.log(coordinateList.getCenter()); 57 } 58 59 function main () { 60 this.document.getElementById('atk').addEventListener('click', { 61 handleEvent: handleClick, 62 length: 3, 63 dimensionList: ['x', 'y'] 64 }, false); 65 } 66 67 main.call(this); 68}.call(this)); 69</script>

Re: hectopascal1013 さん

投稿2018/09/24 01:05

編集2018/09/24 14:19
think49

総合スコア18156

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

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

hectopascal1013

2018/09/24 02:36

ご提示ありがとうございます。 私が理解できる範囲をだいぶ超えているので、これからじっくりと中身を確認して、身につけていきたいと思います。 大分無駄が多かった点を省いてくださり感謝します。 2つに方向性を分けて書いていただいたのもありがたかったです。 また理解しつつお聞きしたい点も出てくると思いますので引き続きよろしくお願い申し上げます。
think49

2018/09/24 14:26

一応、 クラス化したコードを追記しておきました。 最も、汎用性を重視したので、多少コードは長いです。 - 論理エラーを検出できる - 三角形だけでなく、N角形を定義出来る - x,y座標だけでなく、z座標も定義出来る - iterable なオブジェクトを引数にとれる
hectopascal1013

2018/09/26 23:14

沢山書いて頂いてたのに、お礼が遅くなりまして申し訳ありません。 Firefoxの方から更新してコメントできてたと思ってましたができてませんでした。。;; reduceやmapもまだ理解してない初めて半週の私には内容てんこ盛りで、まだまだ理解していない部分もありますが、一通り試させていただいて、おしゃって頂いてる事の1割もわかっているかは怪しいですが、これからじっくり参考にさせていただきます! 本当にありがとうございました。
guest

0

5角形をどうやって3角形に分割しようとしているのかわかりませんが、
arrに3角形の頂点が入っているので、それを使ってはどうでしょうか。

javasript

1const gx = arr.reduce((a, c)=> a + c.x, 0) / 3; 2const gy = arr.reduce((a, c)=> a + c.y, 0) / 3;

Array::reduce


余談ですが、現状のロジックを私であれば以下のように作ります。
もし、参考になる部分があればどうぞ。

JAVASCRIPT

1function atk(){ 2 const createInputElement = (id, value) =>{ 3 const input = document.createElement('input'); 4 input.type = 'number'; 5 input.size = 10; 6 input.id = id; 7 input.placeholder = id; 8 input.setAttribute('value', value); 9 return input; 10 } 11 12 const fm = document.getElementById('fm'); 13 fm.innerHTML=""; 14 15 const arr = []; 16 for(let k=0; k<3; k++) { 17 const x = (Math.random()*1000); 18 const y = (Math.random()*1000); 19 arr.push({x, y}); 20 21 const inputX = createInputElement(`x${k}`, x); 22 const inputY = createInputElement(`y${k}`, y); 23 24 const div = document.createElement('div'); 25 div.innerHTML = `p${k}(${inputX.outerHTML},${inputY.outerHTML})`; 26 fm.append(div); 27 } 28 29 const gx = arr.reduce((a, c)=> a + c.x, 0) / 3; 30 const gy = arr.reduce((a, c)=> a + c.y, 0) / 3; 31 const g = [gx, gy] 32} 33

投稿2018/09/23 13:50

退会済みユーザー

退会済みユーザー

総合スコア0

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

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

hectopascal1013

2018/09/23 22:27

お早いお返事ありがとうございます。返事遅くなってすいません。 なるほど〜。すでにあるarrを利用するにはreduceを使うのですか。 超がつく初心者の私は挙動が思い通りにならないため使用を敬遠してましたが、 こうしてみると簡潔に書けるためにイイですね。勉強します。
guest

0

変数が配列になっても良ければこんな感じでどうでしょう??
ご指定の箇所をfor文で短くしたバージョンです|ー゚)

js

1let x = []; 2let y = []; 3 4for(let i = 0; i < 3; i++){ 5 x[i] = parseFloat(document.getElementById('x' + i).value); 6 y[i] = parseFloat(document.getElementById('y' + i).value); 7} 8 //g 9const g = []; 10const gx = (x[0]+x[1]+x[2])/3; 11const gy = (y[0]+y[1]+y[2])/3; 12g.push(gx); 13g.push(gy); 14alert(g);

投稿2018/09/23 13:44

退会済みユーザー

退会済みユーザー

総合スコア0

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

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

hectopascal1013

2018/09/23 22:29

haru_hime様ありがとうございます。 多分私がやりたかった意図を、一番くんでくださったお答えなのではないかと思います。 xとyで配列を分けるというのは初心者にもわかりやすいやり方ですね。 一度にまとめて書こうとすると混乱もするし、間違いも犯しやすいので、練習に取り入れたいと思います。
hectopascal1013

2018/09/24 02:30

すいません。追加質問です。 let x = []; let y = []; をつけ忘れたパターンは自分で試しましたが、alertでgを出すことに失敗しました。 コレをつけないことで表示されなくなってしまう理由ってなんでしょうか?
退会済みユーザー

退会済みユーザー

2018/09/26 13:40

let x = []; let y = []; が無い時にalertが作動しないということでしょうか? 配列は宣言しなかった場合 x[0] = 10; は、 let x; x[0] = 10 みたいな事をした感じになるんではないかなと勝手に予想してやってます。 もちろんエラーが出ますのでそれ以降の処理は中断されます。 for分の直前や、for文の中の先頭に console.log("ここまで動いた:目印"); を追加して動かしてみるとどこで止まったのかなどが目に見えるので理解しやすいかと思います。 ちなみに、ボタンを押して表示が更新されるのはボタンを押すたびにエラーが出る x[i] = parseFloat(document.getElementById('x' + i).value); の行まで実行されるからです。 変数の宣言については、あまり深く考えた事が無かった(←趣味とはいえ反省してますw)ので詳しい方にバトンパスです|ー゚) クロージャ スコープ スコープチェーンなどの検索用語で色々出てきそうなので勉強してきます! 良い機会感謝です(n*´ω`*n)
hectopascal1013

2018/09/26 23:17

>クロージャ スコープ スコープチェーンなど 私も勉強します。 console.logやalertの確認はするようにします。 変数宣言については他の質問で同じようにご指摘頂いた事でまた一歩、少しですが理解が深まりました。 私の方でまだまだ基礎も確立していない段階ですので、おかしな質問だったらすいません。 ご回答ありがとうございます。
guest

0

ベストアンサー

私が作るならこんな感じの設計にしますね。

javascript

1cconst indexes = [0, 1, 2]; 2const dimensions = ['x', 'y']; 3 4// 中身全体を毎回生成する意味はないので、コンテンツ読み込み完了後一度だけHTML更新。 5// 初回起動時に要素を作るため、微妙に挙動が変わっていますが…… 6document.addEventListener('DOMContentLoaded', ()=>{ 7 function createInput(dimension, index){ 8 return Object.assign( 9 document.createElement('input'), 10 { 11 type: 'number', 12 sixe: 10, 13 id: `${dimension}${index}`, 14 placeholder: `${dimension}${index}`, 15 value: Math.random()*1000 16 } 17 ); 18 } 19 indexes.forEach(i=>{ 20 const inputs = dimensions.map(d=>createInput(d, i)); 21 [ 22 document.createTextNode(`p${i}(`), 23 inputs[0], 24 document.createTextNode(','), 25 inputs[1], 26 document.createTextNode(')'), 27 document.createElement('br') 28 ].forEach(el=>document.getElementById('fm').append(el)); 29 }); 30}); 31function atk(){ 32 // 値を取得 33 const val = dimensions.map(d=>{ 34 return indexes 35 .map(i=>document.getElementById(`${d}${i}`).value) 36 .map(v=>parseFloat(v)); 37 }); 38 // 重心を計算 39 const g = val.map(v=> 40 v.reduce((prev, crnt)=>prev + crnt) / v.length 41 ); 42 alert(g); 43}

若干、挙動を私好みに変えてしまいましたが。

56行くらい→ 43行くらいと、行数はあまり短縮していませんが、コードの見通しが良くなったと思いませんか?

投稿2018/09/23 12:55

R.Mizukami

総合スコア1077

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

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

hectopascal1013

2018/09/23 22:33

>私が作るならこんな感じの設計にしますね。 これは素晴らしい。私が気づいていなかったことまでご指摘いただきありがとうございます。 一人でやっているため、なかなか指摘を受けることもなかったので、こういう視点の組み方は非常に勉強になります。まだまだ、舐める程度で、理解には追いつかない点も多いですが、mapやredeceもうまく使えるようになりたいです。
guest

0

javascript

1function atk() { 2 var fm = document.getElementById('fm'); 3 fm.innerHTML = ""; 4 5 var arr = [], points = []; 6 7 for (k = 0; k < 3; k++) { 8 var inputX = document.createElement('input'); 9 inputX.type = 'number'; 10 inputX.size = 10; 11 inputX.id = `x${k}`; 12 inputX.placeholder = `x${k}`; 13 14 var inputY = document.createElement('input'); 15 inputY.type = 'number'; 16 inputY.size = 10; 17 inputY.id = `y${k}`; 18 inputY.placeholder = `y${k}`; 19 inputY.value = `y${k}`; 20 21 var br = document.createElement('br'); 22 23 var before = document.createTextNode(`p${k}(`); 24 var comma = document.createTextNode(','); 25 var after = document.createTextNode(')'); 26 27 fm.append(before); 28 fm.append(inputX); 29 fm.append(comma); 30 fm.append(inputY); 31 fm.append(after); 32 fm.append(br); 33 34 var p = (Math.random() * 1000); 35 var q = (Math.random() * 1000); 36 37 document.getElementById(`x${k}`).value=p; 38 document.getElementById(`y${k}`).value=q; 39 arr.push({x:p,y:q}); 40 41 points.push({x:parseFloat(document.getElementById('x' + k).value), y:parseFloat(document.getElementById('y' + k).value)}); 42 } 43 44 //g 45 const g = []; 46 const gx = (points[0].x + points[1].x + points[2].x) / 3; 47 const gy = (points[0].y + points[1].y + points[2].y) / 3; 48 g.push(gx); 49 g.push(gy); 50 alert(g); 51} 52

投稿2018/09/23 12:25

編集2018/09/23 12:39
kaba

総合スコア314

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

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

hectopascal1013

2018/09/23 22:34

pushの時点でparseFloat使えるんですね。目からウロコです。ありがとうございます。 皆さん書き方が簡潔で美しいので、それを真似できるようにがんばります。
guest

あなたの回答

tips

太字

斜体

打ち消し線

見出し

引用テキストの挿入

コードの挿入

リンクの挿入

リストの挿入

番号リストの挿入

表の挿入

水平線の挿入

プレビュー

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

ただいまの回答率
85.50%

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

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

質問する

関連した質問