javascriptの綺麗な書き方

Question

前提・実現したいこと

配列で遊んでいるのですが、参考にいろんな書き方を教えてもらえますか。
やっていることは、配列のチャンクをとって、各チャンクの最大値を取っています。
ただ、余剰も残さず最大値を引っ張っています。

該当のソースコード

const array1 = [1,2,3,4,5,6,7,8,9,10,11]

const num = 3;
const array2 = new Array();

let i = array1.length;

for(var n = 0 ; n < i ; n++) {
    
    if(array1.length < num ) {
            memo = Math.max(...array1);
            array2.push(memo);
            i = 0;
    } else {
        let memo = array1.splice(0, num);
            memo = Math.max(...memo);
            array2.push(memo);
    }
}

console.log('array2 = ' + array2)

補足

上のコードは期待した通りに動くのは動いてます。

Comments

[stdio]

なら自分で調べて遊びましょう。そうしないといつまで経っても成長はしませんよ。

[退会済みユーザー]

そうですね。他の人たちが、どう言う風に書くか興味があって聞いたんですが、それですらダメみたいですね。

[stdio]

一般の人からしたら出来たらなんでも良いと思われている世界でその質問は無しですよ。
配列の書き方なんて言語によってコロコロ変わりますし、javascriptのやり方も人それそれです。
ショートコードが書きたいのか、エラーを嫌ってしっかり型確認と型変換を欠かさず行う人とか色々いますよ。

[退会済みユーザー]

提示されているコードは JavaScript っぽくないから、他言語からの人ですかね？おれはこういう質問ワリと好きｗ

[退会済みユーザー]

おっしゃる通りぶっちゃけるとjavascriptはあんまり触ったことはないですw
ついでにプログラムに触レたのも数年ぶりで、しっちゃかめっちゃかなコードで、時間を置いた上で頂いた指摘も踏まえて読み直すとウンコードで恥ずかしいですね。ウンコードに掲載されたいです。

Answer 1

質問文をレビューするとこんな感じです。

• 細かいですが空配列は[]を使って宣言しましょう。
• ループの継続条件は一目で読み取れるようにしましょう、詳しくは後述
• for文の中身はifとelseで同じ事やってますね。ダサいので共通化しましょう、詳しくは後述

for文の第二節はループの継続条件です。
質問文では変数nの数値が0から始まり、変数iを超えた場合に終了すると書かれています。
array1の要素数は11なので、１００人のエンジニアに読ませれば全員が「ああ、この場面では11回ループを回したいのね」と判断をするでしょう。

実際にやってる事はガンガンarray1の要素数が取り除かれて、
要素数が2以下になったら突如i = 0が実行されてループが切れます。
i = 0にするくらいならbreak文使えよとなるのですが
そもそも11回ループを回すと書かない方が良いです。

また、if文の中身は[1, 2].splice(0, 3)がエラーを出すのを懸念しているのでしょうか？
これの結果は元気に[1, 2]を返しますし、元の配列は空配列になります。
つまりif文による分岐は不要です。

この辺の事情から変数i,nをそもそも使わず、
array1をガンガン削りますよという宣言の元、
array1.length != 0みたいな式に変更した方がわかりやすくなるでしょう。

上記を加味して書き直すとこうなります。

const array1 = [1,2,3,4,5,6,7,8,9,10,11];
const num = 3;
const array2 = [];

while (array1.length != 0) {
  array2.push(Math.max(...array1.splice(0, num)));
}
console.log(array2);
// [3, 6, 9, 11]

---

ちなみにLodashを使えばワンライナーです。

_.chain(1)
  .range(12)
  .chunk(3)
  .map(its => Math.max(...its))
  .tap(console.log)
  .value();
// [3, 6, 9, 11]

Comments

[miyabi-sun]

今回はwhile文の中身を一撃で書きましたが、
この辺が評価が分かれるところで、
何か一時的に`tmpArray`みたいな配列を宣言して受ける程度に派生するでしょうが、
ALGOLらしく解くなら大抵はこの形に落ち着くと思います。

[退会済みユーザー]

ありがとうございます。とっても参考になります。Lodashってのがあるんですね。面白いですね。
ifの中身はwhileを使わずに書いたため、おっしゃる通りエラーが返ってくるので、書いていました。

[miyabi-sun]

回答書いてる最中に理解したのですが、
エラーを消すために`i = 0`を入れたというのがより正しい形ですね。
少し回答を修正して対応します。

[退会済みユーザー]

ご丁寧にありがとうございます。^^

Answer 2

コーディング方法

おそらく、コーディングの手順に問題があります。
他言語経験者のようですが、「JavaScript固有の機能を知らずに無駄な書き方をしてしまった」というより「場当たり的にコードを付け加え続けた為にアルゴリズム上の無駄を生産してしまった」という印象を持ちました。
「アルゴリズム上の無駄」はコードを書く前に熟考して排除すべきなので、コーディング前の準備が必要と感じます。

• 大体のロジックを紙に書きだす
• フローチャートを作る

改善点

未出の改善点としては、配列の昇順ソートが保証されているのなら、+-3でindex値を走査すれば良いでしょう。

const input = [1,2,3,4,5,6,7,8,9,10,11], results = [], l = input.length, diff = l % 3;
let i = l - 1;

results.push(input[i]);

if (diff) {
  results.unshift(input[i = i - diff]);
}

while ((i -= 3) > -1) {
  results.unshift(input[i]);
}

console.log(results); // [3, 6, 9, 11]

配列の昇順ソートが保証されていないのなら、毎回、index=0から最大値を求める無駄を排除するコードを書けます。

const input = [1,2,3,4,5,6,7,8,9,10,11], results = [];
let max = Math.max(...input.slice(0, 3));

results.push(max)
for (let i = 3, len =  input.length; i < len; i += 3) {
  results.push(max = Math.max(max,...input.slice(i, i + 3)));
}

console.log(results); // [3, 6, 9, 11]

コードを書きましたが、前述の通り、omaru3 さんの場合は「他人の良いコードの書き方を盗む」よりも「一からアルゴリズムを考えてコーディングする」が必要な段階だと私は思います。

Re: omaru3 さん

Answer 3

最短？

console.log([1,2,3,4,5,6,7,8,9,10,11].reduce((a,b,i)=>(i=i/3|0,(a[i]||0<b?a[i]=b:0),a),[]));

Comments

[退会済みユーザー]

92文字ですか！最短ですね。ありがとうございます。

Answer 4

((r,j) => r.reduce((a,b,i,g) => !(i % j) ? a.concat([g.slice(i,i+j)]) : a, []))([1,2,3,4,5,6,7,8,9,10,11], 3).map(e=>Math.max(...e));

やっていることは、範囲指定した配列から最大値を抜き出して、別の配列に突っ込んでいます。

やっていることは、配列のチャンクをとって、各チャンクの最大値を取っている、です。

Comments

[退会済みユーザー]

凄いコードですね。ありがとうございます。

[papinianus]

綺麗じゃないからマイナスなのか。
> 参考にいろんな書き方
に答えたつもりです。

[退会済みユーザー]

このコードに刺激されて短く書きました。評価しておくね。

Answer 5

php 脳が書くとこんな感じ

const array1 = [1,2,3,4,5,6,7,8,9,10,11];
const array2 = [];
const num = 3;
let count = 0;

array1.forEach((val, key)=>{
  if(key % num === 0){
    count = (key === 0) ? 0 : count + 1;
    array2[count] = 0;
  }
  array2[count] = array2[count] > val ? array2[count] : val;
});
console.log(array2);