Ревью функции по удалению пользователей из массива по имени?

Question

[sashadobrota]

Допустим, что у меня есть вот такой массив пользователей:

var users = [
  {
    name: 'Kelly',
    age: 30
  },
  {
    name: 'Alice',
    age: 18
  },
  {
    name: 'Martin',
    age: 23
  },
  {
    name: 'Jack',
    age: 15  
  },
  {
    name: 'Oustin',
    age: 18
  }
];

И вот такая функция для удаления пользователя:

function removeUser(name, arr) {
  var deleteUser;
  
  for (var i = 0, len = arr.length; i < len; i += 1) {
    if (arr[i].name === name) deleteUser = i;
  }
  
  if (deleteUser === undefined) return; 
  
  arr.splice(deleteUser, 1);
}

Что скажете по-поводу реализации данной функции и стилю написания, годится? Стараюсь писать как можно более понятный код. Можно как-то более элегантно сделать это? Использование map, filter и прочее не предлагать, так как только изучаю js и хочется пока не трогать продвинутые методы по работе с массивами. Спасибо.

Answer 1

[Алексей A]

Ох, с чего бы начать...
1. Сразу бы отметил for (var i = 0, len = arr.length; i < len; i += 1). Так как у вас инкримент происходит каждый раз на единицу, то вместо i += 1 лучше написать i++.
2. В var deleteUser; и deleteUser = i; смысла нет, так как область видимости переменной var i уже будет внутри функции, а не цикла (в ECMAScript 2015 есть чудесная вещь let ).
3. Нет смысла и в if (deleteUser === undefined) return;. Это лишние проверки того, что уже известно. Делайте действие сразу.

for (var i = 0, len = arr.length; i < len; i++) {
    if (arr[i].name === name) arr.splice(i, 1);
  }

4. Этот цикл будет продолжаться даже если пользователь уже найден. Так как вы не удаляете, как я вижу, массово, а только одного пользователя - делаем прерывание цикла.

for (var i = 0, len = arr.length; i < len; i++) {
    if (arr[i].name === name){
      arr.splice(i, 1);
      break; /* или сразу return; */
    }
  }

Comments

[sashadobrota]

Что касается инкремента - это мой личный выбор использовать такую форму записи. За всё остальное - большое спасибо. Особенно за 4-ый пункт, не сообразил.

[Александр Аксентьев]

а если в массиве два человека с удаляемым именем?)

[sashadobrota]

Александр Аксентьев: пока удаляем только первого )

[Дмитрий Беляев]

sashadobrota: Добавлю. Чем больше массив, тем медленнее работает splice, для удаления одного элемента или элементов идущих подряд вполне подойдет, а вот если надумаете удалять несколько пользователей за раз, то они вполне вероятно будут идти в произвольном месте массива и код придется несколько усложнить для оптимизации