Как улучшить код?

Question

[Artem Pavliienko]

Всем привет. Мне нужно было сделать тестовое задание. Дан массив, нужно выбрать 2 самых больший значения и перемножить их.
Вот код:

function adjacentElementsProduct (inputArray) {
    let adjacent = inputArray
        .sort()
        .slice(-2)
        .reduce((total, num) => total * num);
    return adjacent;
}

adjacentElementsProduct( [-1, -2] ); //2
adjacentElementsProduct( [5, 1, 2, 3, 1, 4] ); // 20
adjacentElementsProduct( [1, 2, 3, 0] ); // 6
adjacentElementsProduct( [9, 5, 10, 2, 24, -1, -48] );  // 45
adjacentElementsProduct( [1, 0, 7, 7, 2] );  // 49

Мне ответили что у меня код перегружен. Подскажите пожалуйста, как его можно улучшить

Comments

[gleendo]

// Нормальный у вас код. Сами они перегружены
const adjacentElementsProduct  = arr => arr.sort().slice(-2).reduce((a, b) => a * b);

[Anton Filippov]

iamevg_, у вас тоже перегружен, и вы просто изменили форму написания того, что было у тс)

[gleendo]

flppv, я и не говорил что я написал другую реализацию...

Answer 1

[0xD34F]

Что значит "перегружен"? Как по мне - так нормально.

Конечно, при желании можно и покороче записать.

const adjacentElementsProduct = arr => arr
  .sort((a, b) => a - b)
  .slice(-2)
  .reduce((acc, n) => acc * n, 1);

А вот то, что выдаёт результаты неправильные - это косяк. Четвёртый пример, максимальные элементы - 10 и 24, произведение 240, а у вас 45. Сортировка по умолчанию сравнивает элементы как строки, поэтому максимальными у вас оказываются 5 и 9. Ещё с пустым массивом всё плохо будет.

Answer 2

[youngmysteriouslight]

Насколько я представляю, если не вводить дополнительные переменные, то только

function adjacentElementsProduct (inputArray) {
    return inputArray
        .sort()
        .slice(-2)
        .reduce((total, num) => total * num);
}

Если с доп. переменными, то

function adjacentElementsProduct (inputArray) {
    const [ x, y ] = inputArray
        .sort()
        .slice(-2);
    return x * y;
}

Ну и как намекнул iamevg_, можно поиграться с пробельными символами.
P.S. учтите, что для Вашей задачи можно выбрать более оптимальный алгоритм.

Comments

[Андрей]

Тогда можно и slice убрать

[youngmysteriouslight]

я так понимаю, Вы говорите о троеточии. Но как это сделать без reverse?

[Андрей]

Вот так будет выглядеть правильный вариант с sort, если убрать всё лишнее:

function adjacentElementsProduct(inputArray) {
	const [x, y] = inputArray.sort((a, b) => b - a);
	return x * y;
}

Answer 3

[profesor08]

Твой код при определенных обстоятельствах сломается. Укажи пустой массив и словишь ошибку. Далее, зачем тебе сортировать массив и выполнять какие-то действия, если в нем будет только один элемент? Зачем тебе сортировать массив, выполнять slice, выполнять reduce, если в нем только два элемента? А что, если в массиве будут не цифры, а буквы или еще какой-то мусор? А что, если я захочу выбрать не два, а три или пять элементов, что, каждый раз лезть в функцию и менять ее код, или делать дубликаты под все варианты? Твое решение только решает задачу в лоб, шаг влево или шаг вправо, и твой код превращается в тыкву.