配列の列数が可変のとき、someの結果に処理を書きたい

Question

配列の列数が可変のとき、取り出す方法について。
部屋数が階ごとに可変です。
部屋番号から合う鍵を探し、かつ配列のキーも取得したい場合です。
以下を御覧ください。

const keys = [
  {"keyno":"A1","room1":"102","room2":"103"}, 
  {"keyno":"B2","room1":"201","room2":"202"},
  {"keyno":"C3","room1":"301","room2":"302","room3":"303"},
  {"keyno":"D4","room1":"401"},
];


var reList = [];

var result = keys.filter(x => Object.keys(x).some(key => /^room/.test(key) 
                                                       && x[key] === "102"
                                                       && reList.push({"keyno":x.keyno ,"room":key,"room_no":x[key]})));

result = keys.filter(x => Object.keys(x).some(key => /^room/.test(key) 
                                                       && x[key] === "201"
                                                       && reList.push({"keyno":x.keyno ,"room":key,"room_no":x[key]})));

result = keys.filter(x => Object.keys(x).some(key => /^room/.test(key) 
                                                       && x[key] === "202"
                                                       && reList.push({"keyno":x.keyno ,"room":key,"room_no":x[key]})));

result = keys.filter(x => Object.keys(x).some(key => /^room/.test(key) 
                                                       && x[key] === "303"
                                                       && reList.push({"keyno":x.keyno ,"room":key,"room_no":x[key]})));

result = keys.filter(x => Object.keys(x).some(key => /^room/.test(key) 
                                                       && x[key] === "401"
                                                       && reList.push({"keyno":x.keyno ,"room":key,"room_no":x[key]})));

console.log(reList);

結果：

[
  { keyno: 'A1', room: 'room1', room_no: '102' },
  { keyno: 'B2', room: 'room1', room_no: '201' },
  { keyno: 'B2', room: 'room2', room_no: '202' },
  { keyno: 'C3', room: 'room3', room_no: '303' },
  { keyno: 'D4', room: 'room1', room_no: '401' }
]

鍵の番号、部屋の配列上のキー、検索条件としたroom_noが取得できましたので、望む結果は取得できました。

しかし、someの条件部にpushを書いているのが、違和感がありまして。。
push処理部分をもう少しまともな書き方ができないものが、ご教授いただきたいです。

Answer 1

ダメな方向に行ってます。

リスト操作の関数はそれぞれのやりたいことに責任を持つものです。
JavaScriptのArray.prototypeからめぼしいものを抜き出すとこんな感じ

• map(数学用語): 配列の各要素を加工したい
• filter: 特定用途の要素のものだけを濾し器で取り出したい
• find: 一つだけ抜き出したい
• forEach(each): 「それぞれ(直訳)」を処理する
• reduce(数学用語): 配列を減らして整理する、畳み込み

まぁ英語でフィルタリングしまーすと言いながら、他の配列にpushしてるので
このコードを読んだ人が貴方を「こいつ大嘘吐きのペテン野郎だな」と評価するでしょう。
処理するならforEachを使うべきです。

しかし、重複してるコピペコードをよく見てください。
変更箇所は"102", "201", "202", "303", "401"の部分だけですよね？
ならば最も適切なのは["102", "201", "202", "303", "401"]というルームナンバーリストという配列からmapでキー情報を取り出す事になります。

これでコピペコード郡が一掃されます。
それではいきましょう。

const keys = [
  {"keyno":"A1","room1":"102","room2":"103"}, 
  {"keyno":"B2","room1":"201","room2":"202"},
  {"keyno":"C3","room1":"301","room2":"302","room3":"303"},
  {"keyno":"D4","room1":"401"},
];

// このように欲しい鍵情報の配列を準備したが
// 直接["102", "201", "202", "303", "401"].map(fn)とやっても良い
const rooms = ["102", "201", "202", "303", "401"];
const result = rooms
  .map(room =>
    keys.filter(x =>
      Object.keys(x).some(key => /^room/.test(key) && x[key] === room)
    ).map(x => ({
      keyno: x.keyno,
      room: Object.keys(x).find(key => /^room/.test(key) && x[key] === room),
      room_no: room,
    }))
  ).reduce((arr, it) => [...arr, ...it]);

// 結果表示
console.log(JSON.stringify(result, null, 2));
// [
//   {"keyno": "A1", "room": "room1", "room_no": "102"},
//   {"keyno": "B2", "room": "room1", "room_no": "201"},
//   {"keyno": "B2", "room": "room2", "room_no": "202"},
//   {"keyno": "C3", "room": "room3", "room_no": "303"},
//   {"keyno": "D4", "room": "room1", "room_no": "401"}
// ]

reduce((arr, it) => [...arr, ...it])はfilterがベースになっているので、配列+配列となり、二次元配列になってしまったので潰す目的でこれにしました。

なのでこれは、最初の段階のリファクタリングでしかありません。

---

続いて本当にfilterが妥当なものなのかを考える必要があります。
before、afterのデータとコードを見る限りfilterである意味が微塵もありません。

一つの鍵で複数の部屋を開けられる(マスターキー？)という時点で仰天ですが、
その割には大した数のドアは開けられませんし、掃除してくれる方に対するセキュリティなのかな？
まぁいいや、清掃担当用のマスターキーと仮定しましょう。

もし一つの部屋が複数のマスターキーで開けられて、
全ての鍵を列挙しなければならないのであれば別ですが、
一つの部屋が一つのマスターでしか開けられない前提ならばfilterは混乱の元でしかないのでfindにしてしまいましょう。

const keys = [
  {"keyno":"A1","room1":"102","room2":"103"}, 
  {"keyno":"B2","room1":"201","room2":"202"},
  {"keyno":"C3","room1":"301","room2":"302","room3":"303"},
  {"keyno":"D4","room1":"401"},
];
const rooms = ["102", "201", "202", "303", "401"];
const result = rooms.map(room => {
  validKey = keys.find(x =>
    Object.keys(x).some(key => /^room/.test(key) && x[key] === room)
  )
  if (validKey) {
    return {
      keyno: validKey.keyno,
      room: Object.keys(validKey).find(key => /^room/.test(key) && validKey[key] === room),
      room_no: room,
    }
  }
});

console.log(JSON.stringify(result, null, 2));

結果
[
  {
    "keyno": "A1",
    "room": "room1",
    "room_no": "102"
  },
  {
    "keyno": "B2",
    "room": "room1",
    "room_no": "201"
  },
  {
    "keyno": "B2",
    "room": "room2",
    "room_no": "202"
  },
  {
    "keyno": "C3",
    "room": "room3",
    "room_no": "303"
  },
  {
    "keyno": "D4",
    "room": "room1",
    "room_no": "401"
  }
]

前回の質問でmaisumakunさんの短い回答があり、あれも素敵な回答ですが、
ワンライナーでゴリゴリと一致するroom名を探しているので、
あちこちで同じ処理を書く羽目になってしまっています。

コピペはバグの温床で技術的負債です。
対応策は2つ

• 共通の処理を関数化して取り出す
• 序盤でkeys.map()等を使い、予め使いやすい値に加工しておく

この辺を後の課題として残しておきます。
頑張って下さい。

Comments

[退会済みユーザー]

ありがとうございました。
別のコメントにも書きましたが、配列を組み直す方向で進めます。

Answer 2

もとのデータが扱いにくいので展開しときます。

const keys = [
  {"room1":"102","room2":"103","keyno":"A1"}, 
  {"keyno":"B2","room1":"201","room2":"202"},
  {"keyno":"C3","room1":"301","room2":"302","room3":"303"},
  {"keyno":"D4","room1":"401"},
];

const result = keys.map( x => {
  const {keyno, ...rooms} = x;
  return Object.entries(rooms).map(r => ({keyno , roomName : r[0] , roomNo : r[1]}));
}).flat();


console.log(result);
console.log(result.find(x => x.roomNo === '103').keyno)

[
  { keyno: 'A1', roomName: 'room1', roomNo: '102' },
  { keyno: 'A1', roomName: 'room2', roomNo: '103' },
  { keyno: 'B2', roomName: 'room1', roomNo: '201' },
  { keyno: 'B2', roomName: 'room2', roomNo: '202' },
  { keyno: 'C3', roomName: 'room1', roomNo: '301' },
  { keyno: 'C3', roomName: 'room2', roomNo: '302' },
  { keyno: 'C3', roomName: 'room3', roomNo: '303' },
  { keyno: 'D4', roomName: 'room1', roomNo: '401' }
]
A1

Comments

[miyabi-sun]

これは美しいですね

[退会済みユーザー]

確かに、こうしたほうが、後々の処理が楽ですね。

Answer 3

const keys = [
  {"keyno":"A1","room1":"102","room2":"103"}, 
  {"keyno":"B2","room1":"201","room2":"202"},
  {"keyno":"C3","room1":"301","room2":"302","room3":"303"},
  {"keyno":"D4","room1":"401"},
];
const reList = keys.reduce( ( pre, cur )=>{
	Object.keys( cur ).map( v=> /^room/.test( v ) && pre.push( { "keyno": cur.keyno, "room": v, "room_no": cur[ v ] } ) );
	return pre;
}, [] );

console.dir(reList);

Comments

[kei344]

低評価をされるのはかまいませんが、どの部分に問題があるかはコメントくださいね。当回答および次回以降の回答の改善につながる可能性もあるので、よろしくおねがいします。

[x_x]

わたしは低評価していませんが、「push処理部分」についての質問で、何も言わずにデータ変形だけしていることが評価されてしまったのではないかと思います。
ozwk さんの +1 よりも高かったわけですし。

[kei344]

To: x_xさん
そうならそう書いて欲しいです。あと、他の良い回答の順序を上げる為に下げるのはやめてほしい。

[x_x]

あくまでわたしの想像なので。
順序というか、評価が高いということはそれだけ目に留まりやすいということを言いたかったのです。一番上の回答が間違っていれば何か言いたくなります。

[退会済みユーザー]

ありがとうございました。
チームとしては、kei344さんの提案どおり、配列の組み直しをする方向になりました。

Answer 4

こんにちは

この回答では、以下の2つのコード案を提示します。

• 1. some を使いつつ見通しをよくする。
• 1. keys から別のデータ構造を作り、そこから結果を得る

1. some を使いつつ見通しをよくする。

some を for文のbreakのように使いたいという場面は、ままあるので、ご質問に挙げられている
コードのようなことになるのも、それなりにあり得ることかなと思います。ただし、

someの条件部にpushを書いているのが、違和感がありまして。。

との件は、確かにそう思えますので、some に与える関数を整理した案を挙げます。

まずsome を使って、以下のような関数を作っておきます。（指定された、room_no に該当がない場合の対応も追加しています）

const findByRoomNo = (room_no) => {
   
   let obj = { room_no, error: '該当なし' };
  
   keys.some(e => 
       Object.entries(e).some(([k, v]) => {
         if (v == room_no) {
           obj = { room_no, keyno: e.keyno, room: k };
           return true;
         }
         return false;
       })      
   );
  
   return obj;
};

上記では、someを二重にしていますが、forをbreakで抜けるのと同じ要領で、「見つかった時点で、(内側のループだけではなく外側のループからも） 抜ける」ようになっており、ご質問に挙げられている some の使い方と趣旨は同じです。

上記を使うと、ご質問の本題である結果を得るには、以下のように書けます。

const result = [
  "102", 
  "201", 
  "202", 
  "303", 
  "401", 
  "999"
].map(findByRoomNo);

• 動作確認用ＣｏｄｅＰｅｎ: https://codepen.io/jun68ykt/pen/wvaavaB?editors=0012

2. keys から別のデータ構造を作り、そこから結果を得る

他の回答者様からも、keys から別のデータ構造を作る案がすでに提示されていますが、以下もそれらと同様に、別のデータ構造を得たうえで、そこから結果を得るようにしたものです。以下では、キーと値が

"102" => { keyno: 'A1', room: 'room1' }

という形のエントリを全部屋分持った Mapを作って、そこから結果を得ます。

const roomsMap = keys.reduce((map, keyInfo) => (
  new Map([
    ...map, 
    ...Object
         .entries(keyInfo)
         .filter(([prop]) => /^room\d+/.test(prop))
         .map(([room, room_no]) => [room_no, { keyno: keyInfo.keyno, room }])
  ]))
, new Map);


const result = [
  "102", 
  "201", 
  "202", 
  "303", 
  "401", 
  "999"
].map(noom_no => ({ noom_no, ...(roomsMap.get(noom_no) || { error: '該当なし' }) }));

• 動作確認用ＣｏｄｅＰｅｎ: https://codepen.io/jun68ykt/pen/abOOoRy?editors=0012

以上、参考になれば幸いです。

追記

全体的な修正案ではなく、ご質問のコードの問題の部分

key => /^room/.test(key) && x[key] === "102" && reList.push({"keyno":x.keyno ,"room":key,"room_no":x[key]})

についての本題である、

someの条件部にpushを書いているのが、違和感がありまして。。

との違和感を無くすことのみに目的を限って、この部分への局所的な修正案を挙げますと、上記を以下のようにすれば、その違和感は多少減るのではと思います。

key => {
  if (/^room/.test(key) && x[key] === "102") {
    reList.push( { "keyno":x.keyno ,"room":key,"room_no":x[key] } );
    return true;
  } 
  return false;
}

ただ、上記だと、コード行数が増えますので、行数を減らそうとしてreturnなしのアロー関数で書こうとすると、どこかでreList に push しなければならないので、違和感のあるとお感じの現状にならざるを得ないです。このように、アローの先のreturnを書かずにすむ行数の少ないコードを取るか、読んで違和感のより少ないコードを採るか、という選択が互いにトレードオフになることは、ままあります。（ちなみに、業務でそのような二者の選択があったら、読みやすいほうを私は採ります。）

コード全体の見直しに関しては、この回答の冒頭に挙げた

• 1. some を使いつつ見通しをよくする。
• 1. keys から別のデータ構造を作り、そこから結果を得る

や、他の回答者様からも有用な知見やコードが提示されていますので、そちらを参考にされるとよいかと思います。

Comments

[退会済みユーザー]

ありがとうございました。
チームの方針としては、kei344さんやozwkさんのように、配列を使いやすい形に組み直す方向になりました。
ただ、質問の表題であるsomeの適切な使い方を挙げていただいたjun68yktさんをベストアンサーとさせていただきました。

[jun68ykt]

どういたしまして。
> 配列を使いやすい形に組み直す方向になりました。
との件、よかったですね????

Answer 5

こういうデータの持ち方をしてください

調整版

• codepen

Comments

[yambejp]

const keys = [
{"keyno":"A1","room":["102","103"]},
{"keyno":"B2","room":["201","202"]},
{"keyno":"C3","room":["301","302","303"]},
{"keyno":"D4","room":["401"]},
];

[退会済みユーザー]

説明不足でしたが、keysはよそシステムから飛んでくるデータなので、こちらでは加工できません。
こちらでデータを特定し、room1、room2などの列名を返す必要があります。

[yambejp]

調整しました

[yambejp]

const keys = [
{"keyno":"A1","room1":"102","room2":"103"},
{"keyno":"B2","room1":"201","room2":"202"},
{"keyno":"C3","room1":"301","room2":"302","room3":"303"},
{"keyno":"D4","room1":"401"},
];
const rooms=["102","201","202","303","401"];

const reList=keys.map(key=>
Object.entries(key).filter(ele=>
/^room/.test(ele[0])&&rooms.includes(ele[1])).map(map=>
({keyno:key["keyno"],"room":map[0],"room_no":map[1]})
)
).flat();
console.log(reList);

[退会済みユーザー]

ありがとうございました。
提案いただいたとおり、もらったデータをこちらが使いやすいように、加工する方針になりました。