snowberb – 10-49 Oct 6

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] };
L
lebouwski194d ago
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
Sniberb194d ago
yeah that's way better I often overcomplicate and I dont know how to improve on that
S
ScriptyChris194d ago
@⛄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
L
lebouwski194d ago
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
Sniberb194d ago
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
UU
Unknown User192d ago