snowberb – 10-49 Oct 6

S⛄Snowberb⛄

10/6/2023

Focusing only in code readability, maintainability and quality, which piece of code would you choose? (Both do the same and work) The objective is to calculate the bounds of a point chart Piece 1:

  const bounds = { maxX: 0, minX: 0, maxY: 0, minY: 0 };
  for (const index in dataArray) {
    const { return: returnData, volatility } = dataArray[index];

    if (volatility > bounds.maxX) bounds.maxX = volatility;
    if (volatility < bounds.minX) bounds.minX = volatility;
    if (returnData > bounds.maxY) bounds.maxY = returnData;
    if (returnData < bounds.minY) bounds.minY = returnData;
  }

  const bounds = { maxX: 0, minX: 0, maxY: 0, minY: 0 };
  for (const index in dataArray) {
    const { return: returnData, volatility } = dataArray[index];

    if (volatility > bounds.maxX) bounds.maxX = volatility;
    if (volatility < bounds.minX) bounds.minX = volatility;
    if (returnData > bounds.maxY) bounds.maxY = returnData;
    if (returnData < bounds.minY) bounds.minY = returnData;
  }

Piece 2:

 export const findMinMax = (arr: { low: number; high: number }[]) => {
  if (!arr.length) return [0, 0];

  let min = arr[0].low,
    max = arr[0].high;

  for (let i = 1, len = arr.length; i < len; i++) {
    const v = arr[i].low;

    min = v < min ? v : min;
    max = v > max ? v : max;
  }

  return [min, max];
};

  const ranges_1 = findMinMax(
    dataArray.map(({ return: returnData, volatility }) => ({
      low: returnData || 0,
      high: volatility || 0,
    })) || [],
  );
  const ranges_2 = findMinMax(
    dataArray.map(({ return: returnData, volatility }) => ({
      low: volatility || 0,
      high: returnData || 0,
    })) || [],
  );
  const bounds = { maxX: ranges_2[1], minX: ranges_1[0], maxY: ranges_1[1], minY: ranges_2[0] };

 export const findMinMax = (arr: { low: number; high: number }[]) => {
  if (!arr.length) return [0, 0];

  let min = arr[0].low,
    max = arr[0].high;

  for (let i = 1, len = arr.length; i < len; i++) {
    const v = arr[i].low;

    min = v < min ? v : min;
    max = v > max ? v : max;
  }

  return [min, max];
};

  const ranges_1 = findMinMax(
    dataArray.map(({ return: returnData, volatility }) => ({
      low: returnData || 0,
      high: volatility || 0,
    })) || [],
  );
  const ranges_2 = findMinMax(
    dataArray.map(({ return: returnData, volatility }) => ({
      low: volatility || 0,
      high: returnData || 0,
    })) || [],
  );
  const bounds = { maxX: ranges_2[1], minX: ranges_1[0], maxY: ranges_1[1], minY: ranges_2[0] };

Llebouwski

10/6/2023

what about

const data = [
  { x: 4, y: 8 },
  // etc
];

const xAxis = data.map((point) => point.x);
const yAxis = data.map((point) => point.y);

const bounds = {
  maxX: Math.max(...xAxis),
  minX: Math.min(...xAxis),
  maxY: Math.max(...yAxis),
  minY: Math.min(...yAxis),
};

const data = [
  { x: 4, y: 8 },
  // etc
];

const xAxis = data.map((point) => point.x);
const yAxis = data.map((point) => point.y);

const bounds = {
  maxX: Math.max(...xAxis),
  minX: Math.min(...xAxis),
  maxY: Math.max(...yAxis),
  minY: Math.min(...yAxis),
};

S⛄Snowberb⛄

10/6/2023

yeah that's way better I often overcomplicate and I dont know how to improve on that

SScriptyChris

10/6/2023

@⛄Snowberb⛄ oh, second example is IMO way more difficult to understand and it's longer 😄 Evert gave nice code with Math.min and Math.max instead of conditions Although I think that yours first example, with replaced ifs to mentioned Math methods would be better than Evert's example, because there would only be 1 loop 🤔 And you should avoid using for..in for arrays - use for..of instead and you can destructure stuff directly at the iterator declaration

Llebouwski

10/6/2023

for single loop you could do

const bounds = data.reduce(
  ({ maxX, minX, maxY, minY }, { x, y }) => ({
    maxX: Math.max(maxX, x),
    minX: Math.min(minX, x),
    maxY: Math.max(maxY, y),
    minY: Math.min(minY, y),
  }),
  {
    maxX: Number.NEGATIVE_INFINITY,
    minX: Number.POSITIVE_INFINITY,
    maxY: Number.NEGATIVE_INFINITY,
    minY: Number.POSITIVE_INFINITY,
  }
);

const bounds = data.reduce(
  ({ maxX, minX, maxY, minY }, { x, y }) => ({
    maxX: Math.max(maxX, x),
    minX: Math.min(minX, x),
    maxY: Math.max(maxY, y),
    minY: Math.min(minY, y),
  }),
  {
    maxX: Number.NEGATIVE_INFINITY,
    minX: Number.POSITIVE_INFINITY,
    maxY: Number.NEGATIVE_INFINITY,
    minY: Number.POSITIVE_INFINITY,
  }
);

if it's actually faster you'd have to profile

S⛄Snowberb⛄

10/6/2023

Faster or slower doesnt matter here I know I gave this without context, but taking into account the findMinMax function already exists and is used in other places of the project I do think it's way more readable than the first example, and what is most important, it's rehusable and maintainable but yeah with the min max math functions is the best way of doing this the shortest and the clearest

UUUnknown User

10/8/2023

Message Not Public

Looking for more? Join the community!

Reactiflux

snowberb – 10-49 Oct 6

Join Server

Reactiflux

snowberb – 10-49 Oct 6

Join Server

Want results from more Discord servers?

Add your server