Skip to main content
add link to SO
Source Link
ggorlen
  • 3.8k
  • 2
  • 18
  • 28

Your explanation of what the code does is a little bit unclear. The algorithm you've shown dedupes an array of objects based on ids, choosing the first occurrence of each id. It's not so much that you're choosing "each occurrence" as much as "first occurrence".

You can improve on the linear lookup (.find() loops over the whole acc every time), making the quadratic algorithm a linear one using a Set, which offers constant time lookups. This essentially removes the inner loop in exchange for a bit of extra space and a bit of allocation overhead, likely well worth it on 4k+ items (always benchmark to be sure):

const softwareProducts = [
  {software_id: 1, software_name: "foo"},
  {software_id: 2, software_name: "bar"},
  {software_id: 1, software_name: "ignore this"},
  {software_id: 3, software_name: "baz"},
  {software_id: 1, software_name: "ignore this"},
  {software_id: 3, software_name: "ignore this"},
];
const resultIds = new Set();
const result = [];

for (const {software_id: id, software_name: text} of softwareProducts) {
  if (!resultIds.has(id)) {
    resultIds.add(id);
    result.push({id, text});
  }
}

console.log(result);

.reduce feels a little overworked acting as a .map and .filter. It's usually clearer as an explicit loop as shown above.

Normally, JS uses camelCase, but I assume software_name comes from an API you don't have control over. In that case, it's acceptable to use, but use softwareName if the code is under your control.

acc = [] is unnecessary; just use acc. The default argument is a no-op.

The return value of softwareProducts.reduce needs to be assigned to something. I assume you're doing so in your actual code.


See also Create array of unique objects by property for other approaches.

Your explanation of what the code does is a little bit unclear. The algorithm you've shown dedupes an array of objects based on ids, choosing the first occurrence of each id. It's not so much that you're choosing "each occurrence" as much as "first occurrence".

You can improve on the linear lookup (.find() loops over the whole acc every time), making the quadratic algorithm a linear one using a Set, which offers constant time lookups. This essentially removes the inner loop in exchange for a bit of extra space and a bit of allocation overhead, likely well worth it on 4k+ items (always benchmark to be sure):

const softwareProducts = [
  {software_id: 1, software_name: "foo"},
  {software_id: 2, software_name: "bar"},
  {software_id: 1, software_name: "ignore this"},
  {software_id: 3, software_name: "baz"},
  {software_id: 1, software_name: "ignore this"},
  {software_id: 3, software_name: "ignore this"},
];
const resultIds = new Set();
const result = [];

for (const {software_id: id, software_name: text} of softwareProducts) {
  if (!resultIds.has(id)) {
    resultIds.add(id);
    result.push({id, text});
  }
}

console.log(result);

.reduce feels a little overworked acting as a .map and .filter. It's usually clearer as an explicit loop as shown above.

Normally, JS uses camelCase, but I assume software_name comes from an API you don't have control over. In that case, it's acceptable to use, but use softwareName if the code is under your control.

acc = [] is unnecessary; just use acc. The default argument is a no-op.

The return value of softwareProducts.reduce needs to be assigned to something. I assume you're doing so in your actual code.

Your explanation of what the code does is a little bit unclear. The algorithm you've shown dedupes an array of objects based on ids, choosing the first occurrence of each id. It's not so much that you're choosing "each occurrence" as much as "first occurrence".

You can improve on the linear lookup (.find() loops over the whole acc every time), making the quadratic algorithm a linear one using a Set, which offers constant time lookups. This essentially removes the inner loop in exchange for a bit of extra space and a bit of allocation overhead, likely well worth it on 4k+ items (always benchmark to be sure):

const softwareProducts = [
  {software_id: 1, software_name: "foo"},
  {software_id: 2, software_name: "bar"},
  {software_id: 1, software_name: "ignore this"},
  {software_id: 3, software_name: "baz"},
  {software_id: 1, software_name: "ignore this"},
  {software_id: 3, software_name: "ignore this"},
];
const resultIds = new Set();
const result = [];

for (const {software_id: id, software_name: text} of softwareProducts) {
  if (!resultIds.has(id)) {
    resultIds.add(id);
    result.push({id, text});
  }
}

console.log(result);

.reduce feels a little overworked acting as a .map and .filter. It's usually clearer as an explicit loop as shown above.

Normally, JS uses camelCase, but I assume software_name comes from an API you don't have control over. In that case, it's acceptable to use, but use softwareName if the code is under your control.

acc = [] is unnecessary; just use acc. The default argument is a no-op.

The return value of softwareProducts.reduce needs to be assigned to something. I assume you're doing so in your actual code.


See also Create array of unique objects by property for other approaches.

clarify
Source Link
ggorlen
  • 3.8k
  • 2
  • 18
  • 28

Your explanation of what the code does is a little bit unclear. The algorithm you've shown dedupes an array of objects based on ids, choosing the first occurrence of each id. It's not so much that you're choosing "each occurrence" as much as "first occurrence".

You can speed upimprove on the linear lookup (.find() loops over the whole acc every time) and improve, making the quadratic algorithm to a linear one using a Set, which offers constant time lookups. This essentially removes the inner loop in exchange for a bit of extra space and a bit of allocation overhead, likely well worth it on 4k+ items (always benchmark to be sure):

const softwareProducts = [
  {software_id: 1, software_name: "foo"},
  {software_id: 2, software_name: "bar"},
  {software_id: 1, software_name: "ignore this"},
  {software_id: 3, software_name: "baz"},
  {software_id: 1, software_name: "ignore this"},
  {software_id: 3, software_name: "ignore this"},
];
const resultIds = new Set();
const result = [];

for (const {software_id: id, software_name: text} of softwareProducts) {
  if (!resultIds.has(id)) {
    resultIds.add(id);
    result.push({id, text});
  }
}

console.log(result);

.reduce feels a little overworked acting as a .map and .filter. It's usually clearer as an explicit loop as shown above.

Normally, JS uses camelCase, but I assume software_name comes from an API you don't have control over. In that case, it's acceptable to use, but use softwareName if the code is under your control.

acc = [] is unnecessary; just use acc. The default argument is a no-op.

The return value of softwareProducts.reduce needs to be assigned to something. I assume you're doing so in your actual code.

Your explanation of what the code does is a little bit unclear. The algorithm you've shown dedupes an array of objects based on ids, choosing the first occurrence of each id. It's not so much that you're choosing "each occurrence" as much as "first occurrence".

You can speed up the linear lookup (.find() loops over the whole acc every time) and improve the quadratic algorithm to a linear one using a Set, which offers constant time lookups. This essentially removes the inner loop in exchange for a bit of extra space and a bit of allocation overhead, well worth it on 4k+ items:

const softwareProducts = [
  {software_id: 1, software_name: "foo"},
  {software_id: 2, software_name: "bar"},
  {software_id: 1, software_name: "ignore this"},
  {software_id: 3, software_name: "baz"},
  {software_id: 1, software_name: "ignore this"},
  {software_id: 3, software_name: "ignore this"},
];
const resultIds = new Set();
const result = [];

for (const {software_id: id, software_name: text} of softwareProducts) {
  if (!resultIds.has(id)) {
    resultIds.add(id);
    result.push({id, text});
  }
}

console.log(result);

.reduce feels a little overworked acting as a .map and .filter. It's usually clearer as an explicit loop as shown above.

Normally, JS uses camelCase, but I assume software_name comes from an API you don't have control over. In that case, it's acceptable to use, but use softwareName if the code is under your control.

acc = [] is unnecessary; just use acc. The default argument is a no-op.

The return value of softwareProducts.reduce needs to be assigned to something. I assume you're doing so in your actual code.

Your explanation of what the code does is a little bit unclear. The algorithm you've shown dedupes an array of objects based on ids, choosing the first occurrence of each id. It's not so much that you're choosing "each occurrence" as much as "first occurrence".

You can improve on the linear lookup (.find() loops over the whole acc every time), making the quadratic algorithm a linear one using a Set, which offers constant time lookups. This essentially removes the inner loop in exchange for a bit of extra space and a bit of allocation overhead, likely well worth it on 4k+ items (always benchmark to be sure):

const softwareProducts = [
  {software_id: 1, software_name: "foo"},
  {software_id: 2, software_name: "bar"},
  {software_id: 1, software_name: "ignore this"},
  {software_id: 3, software_name: "baz"},
  {software_id: 1, software_name: "ignore this"},
  {software_id: 3, software_name: "ignore this"},
];
const resultIds = new Set();
const result = [];

for (const {software_id: id, software_name: text} of softwareProducts) {
  if (!resultIds.has(id)) {
    resultIds.add(id);
    result.push({id, text});
  }
}

console.log(result);

.reduce feels a little overworked acting as a .map and .filter. It's usually clearer as an explicit loop as shown above.

Normally, JS uses camelCase, but I assume software_name comes from an API you don't have control over. In that case, it's acceptable to use, but use softwareName if the code is under your control.

acc = [] is unnecessary; just use acc. The default argument is a no-op.

The return value of softwareProducts.reduce needs to be assigned to something. I assume you're doing so in your actual code.

deleted 80 characters in body
Source Link
ggorlen
  • 3.8k
  • 2
  • 18
  • 28

Your explanation of what the code does is a little bit unclear. The algorithm you've shown dedupes an array of objects based on ids, choosing the first occurrence of each id. It's not so much that you're choosing "each occurrence" as much as "first occurrence".

You can speed up the linear lookup (.find() loops over the whole acc every time) and improve the quadratic algorithm to a linear one using a Set, which offers constant time lookups,. This essentially removingremoves the inner loop in exchange for a bit of extra space and a bit of allocation overhead, well worth it on 4k+ items:

const softwareProducts = [
  {software_id: 1, software_name: "foo"},
  {software_id: 2, software_name: "bar"},
  {software_id: 1, software_name: "ignore this"},
  {software_id: 3, software_name: "baz"},
  {software_id: 1, software_name: "ignore this"},
  {software_id: 3, software_name: "ignore this"},
];
const resultIds = new Set();
const result = [];

for (const product{software_id: id, software_name: text} of softwareProducts) {
  if (!resultIds.has(product.software_idid)) {
    resultIds.add(product.software_idid);
    result.push({
      id: product.software_id,
      text: product.software_name
    });
  }
}

console.log(result);

If you're using .reduce feels a little overworked acting as a .map/ and .filter, it's. It's usually clearer as an explicit loop as shown above.

Normally, JS uses camelCase, but I assume software_name comes from an API you don't have control over. In that case, it's acceptable to use, but use softwareName if the code is under your control.

acc = [] is unnecessary; just use acc. The default argument is a no-op.

The return value of softwareProducts.reduce needs to be assigned to something. I assume you're doing so in your actual code.

Your explanation of what the code does is a little bit unclear. The algorithm you've shown dedupes an array of objects based on ids, choosing the first occurrence of each id. It's not so much that you're choosing "each occurrence" as much as "first occurrence".

You can speed up the linear lookup (.find() loops over the whole acc every time) and improve the quadratic algorithm to a linear one using a Set, which offers constant time lookups, essentially removing the inner loop in exchange for a bit of extra space:

const softwareProducts = [
  {software_id: 1, software_name: "foo"},
  {software_id: 2, software_name: "bar"},
  {software_id: 1, software_name: "ignore this"},
  {software_id: 3, software_name: "baz"},
  {software_id: 1, software_name: "ignore this"},
  {software_id: 3, software_name: "ignore this"},
];
const resultIds = new Set();
const result = [];

for (const product of softwareProducts) {
  if (!resultIds.has(product.software_id)) {
    resultIds.add(product.software_id);
    result.push({
      id: product.software_id,
      text: product.software_name
    });
  }
}

console.log(result);

If you're using .reduce as a .map/.filter, it's usually clearer as an explicit loop as shown above.

Normally, JS uses camelCase, but I assume software_name comes from an API you don't have control over. In that case, it's acceptable to use, but use softwareName if the code is under your control.

acc = [] is unnecessary; just use acc. The default argument is a no-op.

Your explanation of what the code does is a little bit unclear. The algorithm you've shown dedupes an array of objects based on ids, choosing the first occurrence of each id. It's not so much that you're choosing "each occurrence" as much as "first occurrence".

You can speed up the linear lookup (.find() loops over the whole acc every time) and improve the quadratic algorithm to a linear one using a Set, which offers constant time lookups. This essentially removes the inner loop in exchange for a bit of extra space and a bit of allocation overhead, well worth it on 4k+ items:

const softwareProducts = [
  {software_id: 1, software_name: "foo"},
  {software_id: 2, software_name: "bar"},
  {software_id: 1, software_name: "ignore this"},
  {software_id: 3, software_name: "baz"},
  {software_id: 1, software_name: "ignore this"},
  {software_id: 3, software_name: "ignore this"},
];
const resultIds = new Set();
const result = [];

for (const {software_id: id, software_name: text} of softwareProducts) {
  if (!resultIds.has(id)) {
    resultIds.add(id);
    result.push({id, text});
  }
}

console.log(result);

.reduce feels a little overworked acting as a .map and .filter. It's usually clearer as an explicit loop as shown above.

Normally, JS uses camelCase, but I assume software_name comes from an API you don't have control over. In that case, it's acceptable to use, but use softwareName if the code is under your control.

acc = [] is unnecessary; just use acc. The default argument is a no-op.

The return value of softwareProducts.reduce needs to be assigned to something. I assume you're doing so in your actual code.

Source Link
ggorlen
  • 3.8k
  • 2
  • 18
  • 28
Loading