JavaScriptの文字列一致で、配列が上書きされてしまいます。

Question

###実現したいこと
下記targetsのうちoksを含むかどうかで区別して、目的であるrを作りたいです。

a1,a2とb1,b2はoksを含むためそれぞれのキーの配列に入れて、
c1,c2はoksを含まないためerrorsに入れる、というのが目的のrになります。

var targets = ['a1','a2','b1','b2','c1','c2'];
var oks = ['a','b'];

// 目的
r = [
  a: ['a1','a2'],
  b: ['b1','b2'],
  errors: ['c1','c2']
];

###発生している問題
後の値しか配列に入りません。
下記のソースコードですが、OKな方はa2やb2しか入らず、エラーの方はc2しか入らないということが問題です。
前の値a1やb1やc1も入れたいのですが、どのようにすべきでしょうか。

###該当のソースコード
.matchを使いoksを含むかどうかを区別しているのですが、うまくいっていないみたいです。

var targets = ['a1','a2','b1','b2','c1','c2'];
var oks = ['a','b'];

let r = [];

targets.forEach( target => {
	
  oks.forEach( ok => {
  
    if ( target.match(ok) ) {
      r[ok] = [];
      r[ok].push( target );
    }else{
      r['errors'] = [];
      r['errors'].push( target );
    }

  });
  
});

console.log( r );

##試したこと
含むかどうかの検証方法として.matchでなく.indexOfを用いましたができませんでした。

    //if ( target.match(ok) ) {
    if ( target.indexOf(ok) != -1) {
      r[ok] = [];
      r[ok].push( target );
    }else{
      r['errors'] = [];
      r['errors'].push( target );
    }

以上のように区別の方法を変えても同じ結果であることから、ループに原因があるのではないかと思慮しています。

ですが、初期化して.pushする、という流れの何がいけないのかがわからず、ここに質問させて頂きました。

解決策についてアドバイス願えましたら幸いです。
宜しくお願い致します。

Answer 1

ループに原因があるのではないかと思慮

target.match() の引数に文字列を格納しています（正規表現でなければなりません）。

oks には、 必ず「最初の１字が格納される」のであれば、「試したこと」に記載されたコードの条件式を target.indexOf(ok) === 0 や target.charAt(0) === ok で試してみてください。
（この２つの関数についても MDN で再確認してください）

追記）
oks の評価は filter() を使ってはどうでしょうか。

1. 求める結果が オブジェクトなので、r={} にする
2. oks.forEach() を tmp = oks.filter( s=>target.indexOf(s)!==-1 ) とする。
3. tmp.length が 1 なら r[tmp[0]], 0 なら r[error]

初期化は r["hoge"] || (r["hoge"] = []);とする

Comments

[LeCreuset]

matchに渡すのが文字列というご指摘には納得です。ですがindexOfの方は文字列で平気ですよね。
ならばループに原因があるという点は間違っていないのではと考えてしまいますが、このあたりいかがでしょうか。

> 必ず「最初の１字が格納される」
というわけではないです。失礼しました。確かに最初の1文字であれば、indexOfは位置が返るのでその書き方も良いかもしれませんね。

[AkitoshiManabe]

追記しました。 r はあくまでも配列でプロパティを付与したいのであれば、追記の 1. は無視してください。

[think49]

To: LeCreuset さん
> ですがindexOfの方は文字列で平気ですよね。

Array#indexOf は「完全一致」で検索しますが、期待している動作は「前方一致」もしくは「部分一致」です。
一方、String#indedxOf は「部分一致」です。

[LeCreuset]

おお、こうですか。できましたできました。
https://jsfiddle.net/p8wmrx7s/
filterや初期化の方法など大変勉強になりました。いつもありがとうございます。

[LeCreuset]

To: think49 さん
なるほど。そのようなことはまったく知りませんでした。ありがとうございます。

Answer 2

こんにちは

ご質問に挙げられているコードの問題点はすでに、他の回答者の方々が指摘されていますので、そちらを参考にして頂ければと思います。この回答では、参考までに、別の書き方を挙げます。reduce とスプレッド構文を使います。

var r = targets.reduce((obj, e) => {
    const key = oks.includes(e[0]) ? e[0] : 'errors';
    obj[key] = [...(obj[key] || []), e];
    return obj;
}, {});

• 動作確認用 CodePen: https://codepen.io/jun68ykt/pen/MWYEoaG?editors=0012

参考になれば幸いです。

補足

もし、 oksの要素が、アルファベット一文字とは限らないのであれば、keyを確定する行を以下のようにします。

const key = oks.find(ok => e.startsWith(ok)) || 'errors';

• 動作確認用 CodePen: https://codepen.io/jun68ykt/pen/eYmGRjo?editors=0012

Comments

[LeCreuset]

申し訳ございません。内容を理解してからご返信しようと思っておりまして…結局reduceのMDNの最初の理解から躓きましたｗ高度なテクニックをありがとうございます。

[jun68ykt]

どういたしまして。
ご質問のコードでは　forEach　が使われていますね。 ということは、「 for 文を使わないで、forEach を使ってみよう」と思いつくことはお出来になるものと思います。ですので、次に 、
「ここは、reduceの使い所だ」
と思いつくようになることは、適度な高さのハードルになると思います。これをクリアするには、次に

・何らかの配列targetsの要素を１個ずつループで取り出して、それらを反映したオブジェクト r を作る。

という場面に出会ったら、　今現在では、

const r = {};

targets.forEach( target => {

と書いてしまうと思いますが、こう書かないで、reduce でうまく書けるか自信がなくても

const r = targets.reduce((obj, e) => {

　　 // TODO: ここにロジックを書く

return obj;
}, {});

まで、ひとまず書いてしまってから、

// TODO: ここにロジックを書く

の部分をどうするか悩むようにします。

はじめは、 reduce でやろうとして、うまくいかないこともあると思いますが、一回でも書き切ることができると、次はもっと楽に書けるようになって、だんだんとモノにできるようになってきます。 reduceを自分の道具にできると、（読みやすさが損なわれることもありますが、）かなり短いコードで済ませることができるようになります。

Answer 3

ここをr[ok]に配列が入ってるかどうかを確認してからすればよいです。

r[ok] = [];

Comments

[LeCreuset]

なるほど。あってもなくても初期化してしまっていたわけですね。

仰るようにlengthで確認のステップを加えてみたののが以下ですが「okがundfinedだ」というエラーにぶちあたってしまいました。

たびたび申し訳ございませんが、この場合の解決策はいかがでしょうか。

if ( target.indexOf(ok) != -1) {
if( ! r[ok].length ){
r[ok] = [];
}
r[ok].push( target );
}else{
if( ! r['errors'].length ){
r['errors'] = [];
}
r['errors'].push( target );
}

[kei344]

「JavaScript 変数 有無」「JavaScript 配列 有無」とかで検索してみてください。

[LeCreuset]

まず「配列 有無」の方は .length であっていますよね？

そして「変数 有無」ですが、これは以下２つ以外ですとどういった方法がいいでしょうか？

シンプルに下記のチェックを挟んでも同じエラーですし、

if ( target.indexOf(ok) != -1) {
if ( r[ok] ) {
if( ! r[ok].length ){
r[ok] = [];
}
}
r[ok].push( target );
}

下記の typeof でも同じエラー、という状態でして。

if ( target.indexOf(ok) != -1) {
if (typeof r[ok] == 'undefined') {
if( ! r[ok].length ){
r[ok] = [];
}
}
r[ok].push( target );
}

[kei344]

最初の物は変数に値が入っているときに、空だったら空配列を入れているが、入っていない時は何もしていない。（そもそも配列があるのに空配列を入れる意味は無い）
後者は変数に値が入っていないときに無いのにもかかわらずlengthプロパティを参照したからエラー。

[LeCreuset]

混乱中です。難しいですね条件…。ありがとうございます。