teratail
回答率
85.47%
質問するログイン新規登録
トップJavaScriptに関する質問スマートな配列処理をしたい

編集履歴

回答編集履歴

5

ちょっと修正

2018/12/28 04:37

投稿

miyabi-sun
miyabi-sun

スコア21158

test CHANGED
@@ -74,6 +74,16 @@
74
74
 
75
75
  );
76
76
 
77
+
78
+
79
+ // あるか無いか分からない場面だと先にfilter使うんだけど、ちょっと冗長かなぁ？
80
+
81
+ shopIds
82
+
83
+ .filter(id => shops.some(it => it.id === id))
84
+
85
+ .forEach(id => shops.find(it => it.id === id).isOpen = false)
86
+
77
87
  ```
78
88
 
79
89
 
@@ -101,6 +111,16 @@
101
111
  // 2: {id: 3, name: "テラ民", isOpen: true}
102
112
 
103
113
  // 3: {id: 4, name: "テラ木屋", isOpen: true}
114
+
115
+
116
+
117
+ // こちらもfilter版、かなりシンプルでスマート！
118
+
119
+ shops
120
+
121
+ .filter(it => shopIds.includes(it.id))
122
+
123
+ .forEach(it => it.isOpen = false)
104
124
 
105
125
  ```
106
126
 

4

コードの修正

2018/12/28 04:37

投稿

miyabi-sun
miyabi-sun

スコア21158

test CHANGED
@@ -130,7 +130,7 @@
130
130
 
131
131
  ...it, isOpen: shopIds.includes(it.id) ? false : it.isOpen
132
132
 
133
- });
133
+ }));
134
134
 
135
135
  console.log(initialisedShops);
136
136
 

3

校生

2018/12/28 04:33

投稿

miyabi-sun
miyabi-sun

スコア21158

test CHANGED
@@ -1,8 +1,32 @@
1
- constで作ったオブジェクトに対して破壊的操作で値をいじるのはあまり良くないのですが…
1
+ constで作ったオブジェクトに対して破壊的操作で値をいじるのはあまり良くありません。
2
2
 
3
- まぁいいや、こういう2つの配列を使う場合はどちらを起点にするかでコードの長さが変わります。
3
+ という訳で両方のケースで書きます。
4
4
 
5
+
6
+
7
+ 今回の質問文には書かれてませんが、
8
+
9
+ 多分この`shopIds`は何らかの事情によりWebサイト上で一時的に準備中にしたいという目的で定義された変数だろうと推測されます。
10
+
11
+ なのでそれ想定で書いてます。
12
+
13
+
14
+
15
+ ---
16
+
17
+
18
+
19
+ プロパティの値に代入はオブジェクトの破壊的な操作になります。
20
+
21
+ 今回のような破壊的操作を伴う場合、戻り値は不要なのでmapは使わずforEachを使うようにしてください。
22
+
23
+ mapを使うと未来の貴方や、他のメンバーは「戻り値を使って何かしたいんだ」と勘違いする要因になるので決して使わないでください。
24
+
25
+
26
+
27
+ また、こういう2つの配列を使う場合はどちらを起点にするかでコードの長さが変わります。
28
+
5
- この際、どちらが短いか両方書いてみてしっくり来る方を選びましょう。
29
+ 両方書いてみてしっくり来る方を選びましょう。
6
30
 
7
31
 
8
32
 
@@ -11,20 +35,6 @@
11
35
 
12
36
 
13
37
  ```JavaScript
14
-
15
- const shopIds = [1, 2];
16
-
17
- const shops = [
18
-
19
- { id: 1, name: "テラ貴族", isOpen: true},
20
-
21
- { id: 2, name: "テラサカバ", isOpen: true},
22
-
23
- { id: 3, name: "テラ民", isOpen: true},
24
-
25
- { id: 4, name: "テラ木屋", isOpen: true},
26
-
27
- ];
28
38
 
29
39
  shopIds.forEach(id =>
30
40
 
@@ -60,11 +70,7 @@
60
70
 
61
71
  shopIds.forEach(id =>
62
72
 
63
- shops
64
-
65
- .find(it => it.id === id)
73
+ shops.find(it => it.id === id).isOpen = false
66
-
67
- .isOpen = false
68
74
 
69
75
  );
70
76
 
@@ -77,20 +83,6 @@
77
83
 
78
84
 
79
85
  ```JavaScript
80
-
81
- const shopIds = [1, 2];
82
-
83
- const shops = [
84
-
85
- { id: 1, name: "テラ貴族", isOpen: true},
86
-
87
- { id: 2, name: "テラサカバ", isOpen: true},
88
-
89
- { id: 3, name: "テラ民", isOpen: true},
90
-
91
- { id: 4, name: "テラ木屋", isOpen: true},
92
-
93
- ];
94
86
 
95
87
  shops.forEach(it => {
96
88
 
@@ -120,25 +112,19 @@
120
112
 
121
113
  オブジェクトの破壊的操作を避け、コピーする想定でいくならこんな感じになります。
122
114
 
115
+ JavaScriptのプロパティ値に代入する方法は破壊的操作なので使えません。
116
+
117
+ mapとシャロコピーを併用する方式になります。
118
+
119
+
120
+
121
+ shopIdsを主軸にすると変更＋変更してないショップ一覧にたどり着くのが困難になりますので、
122
+
123
- どの道ワンライナーで書けますのでこちらの方が良いでしょう。
123
+ shops.mapを主軸にするのが適切でしょう。
124
124
 
125
125
 
126
126
 
127
127
  ```JavaScript
128
-
129
- const shopIds = [1, 2];
130
-
131
- const shops = [
132
-
133
- { id: 1, name: "テラ貴族", isOpen: true},
134
-
135
- { id: 2, name: "テラサカバ", isOpen: true},
136
-
137
- { id: 3, name: "テラ民", isOpen: true},
138
-
139
- { id: 4, name: "テラ木屋", isOpen: true},
140
-
141
- ];
142
128
 
143
129
  const initialisedShops = shops.map(it => ({
144
130
 

2

ツッコミをツッコミの位置へ移動

2018/12/28 03:15

投稿

miyabi-sun
miyabi-sun

スコア21158

test CHANGED
@@ -1,25 +1,3 @@
1
- > 配列shopIdsに格納されたidを持つshopの**isOpenフラグをfalseに書き換える**プログラムを書きました。
2
-
3
-
4
-
5
- ふんふん……
6
-
7
-
8
-
9
- > `shop.isOpen = true;`
10
-
11
-
12
-
13
- 書けてないやん
14
-
15
-
16
-
17
- ---
18
-
19
-
20
-
21
- さて本題、
22
-
23
1
  constで作ったオブジェクトに対して破壊的操作で値をいじるのはあまり良くないのですが…
24
2
 
25
3
  まぁいいや、こういう2つの配列を使う場合はどちらを起点にするかでコードの長さが変わります。

1

良いコードをひらめいたので追記

2018/12/28 02:55

投稿

miyabi-sun
miyabi-sun

スコア21158

test CHANGED
@@ -22,7 +22,13 @@
22
22
 
23
23
  constで作ったオブジェクトに対して破壊的操作で値をいじるのはあまり良くないのですが…
24
24
 
25
- まぁいいや、ワンライナーで書けばこうですかね。
25
+ まぁいいや、こういう2つの配列を使う場合はどちらを起点にするかでコードの長さが変わります。
26
+
27
+ この際、どちらが短いか両方書いてみてしっくり来る方を選びましょう。
28
+
29
+
30
+
31
+ shopIdsを先に使うとこんな感じのコードになります。
26
32
 
27
33
 
28
34
 
@@ -88,13 +94,7 @@
88
94
 
89
95
 
90
96
 
91
- ---
92
-
93
-
94
-
95
- オブジェクトの破壊的操作を避け、コピーする想定でいくならこんな感じになります。
96
-
97
- どちらからいってもワンライナーですね。
97
+ 逆にshopsを使えばこのようなコードになるでしょう。
98
98
 
99
99
 
100
100
 
@@ -114,11 +114,59 @@
114
114
 
115
115
  ];
116
116
 
117
- const initialisedShops = shops.map(it =>
117
+ shops.forEach(it => {
118
118
 
119
- shopIds.includes(it.id) ? {...it, isOpen: false} : {...it}
119
+ if (shopIds.includes(it.id)) it.isOpen = false
120
120
 
121
+ })
122
+
123
+ console.log(shops);
124
+
125
+ // (4) [{…}, {…}, {…}, {…}]
126
+
127
+ // 0: {id: 1, name: "テラ貴族", isOpen: false}
128
+
129
+ // 1: {id: 2, name: "テラサカバ", isOpen: false}
130
+
131
+ // 2: {id: 3, name: "テラ民", isOpen: true}
132
+
133
+ // 3: {id: 4, name: "テラ木屋", isOpen: true}
134
+
135
+ ```
136
+
137
+
138
+
139
+ ---
140
+
141
+
142
+
143
+ オブジェクトの破壊的操作を避け、コピーする想定でいくならこんな感じになります。
144
+
145
+ どの道ワンライナーで書けますのでこちらの方が良いでしょう。
146
+
147
+
148
+
149
+ ```JavaScript
150
+
151
+ const shopIds = [1, 2];
152
+
153
+ const shops = [
154
+
155
+ { id: 1, name: "テラ貴族", isOpen: true},
156
+
157
+ { id: 2, name: "テラサカバ", isOpen: true},
158
+
159
+ { id: 3, name: "テラ民", isOpen: true},
160
+
161
+ { id: 4, name: "テラ木屋", isOpen: true},
162
+
163
+ ];
164
+
165
+ const initialisedShops = shops.map(it => ({
166
+
167
+ ...it, isOpen: shopIds.includes(it.id) ? false : it.isOpen
168
+
121
- );
169
+ });
122
170
 
123
171
  console.log(initialisedShops);
124
172