Как отрефакторить такой код?

Question

[kachurinets]

Использую на проекте такую функцию для удаления полей фильтра:

clearAllControl() {
        if (this.tableParam.region_id) {
            delete this.tableParam.region_id;
            this.searchForm.get('regionName').patchValue(null);
        }
        if (this.tableParam.district_id) {
            delete this.tableParam.district_id;
            this.searchForm.get('districtName').patchValue(null);
        }
        if (this.tableParam.area_id) {
            delete this.tableParam.area_id;
            this.searchForm.get('areaName').patchValue(null);
        }
        if (this.tableParam.city_id) {
            delete this.tableParam.city_id;
            this.searchForm.get('cityName').patchValue(null);
        }
        if (this.tableParam.place_id) {
            delete this.tableParam.place_id;
            this.searchForm.get('cityName').patchValue(null);
        }
        if (this.tableParam.name) {
            delete this.tableParam.name;
            this.searchForm.get('streetName').patchValue(null);
        }
        this.generateAddressesTable(this.tableParam);
    }

Мне не очень нравится реализация данной функции, как можно ее отрефакторить?

Comments

[AngReload]

SmInc, я хотел написать то же самое, поддерживаю.

Answer 1

[0xD34F]

Общую часть похожих кусков кода превращаем в функцию, а то, чем они различаются, будем передавать в эту функцию в качестве параметров:

[
  { del: 'region_id', search: 'regionName' },
  { del: 'district_id', search: 'districtName' },
  { del: 'area_id', search: 'areaName' },
  { del: 'city_id', search: 'cityName' },
  { del: 'place_id', search: 'cityName' },
  { del: 'name', search: 'streetName' },
].forEach(n => {
  if (this.tableParam[n.del]) {
    delete this.tableParam[n.del];
    this.searchForm.get(n.search).patchValue(null);
  }
});

Comments

[kachurinets]

Виктор, давно слежу за твоими ответами, ты самый крутой чувак, которого я знаю!) Спасибо, за помощь)

[Сергей delphinpro]

Еще уменьшим буковок =)

[
  'region', 'district', 'area', 'city', 'place', 'name'
].forEach(n => {
  let id = n+'_id';
  if (this.tableParam[id]) {
    delete this.tableParam[id];
    this.searchForm.get(n+'Name').patchValue(null);
  }
});

[0xD34F]

Сергей delphinpro, так не получится, там какие-то стрёмные соответствия между именами некоторых свойств: cityName повторяется дважды - для city_id и place_id; у streetName в паре почему-то не street_id, а name.

[Сергей delphinpro]

0xD34F, блин, не обратил внимания.. верно.

[Санан :з]

я когда вижу такой код с кучей условий, тоже сразу думаю про циклы и абстракцию

Answer 2

[dollar]

+++

const config = [
  'region', 'district', 'area', 'city',
  {id:'place_id', name:'cityName'}, {id:'name', name:'streetName'},
];

clearAllControl() {
  config.forEach(x=>{
    let id = x.id || (x + '_id');
    let name = x.name || (x + 'Name');
    if (this.tableParam[id]) {
      delete this.tableParam[id];
      this.searchForm.get(name).patchValue(null);
    }
  });
  this.generateAddressesTable(this.tableParam);
}

Comments

[AngReload]

Мне кажется, так тоже делать не стоит.

[dollar]

AngReload, почему же? "Снаружи" - просто перечисление, что вполне удобно. Если его не хватает, то поддерживается расширенный формат.

[dollar]

MaxKorz, тогда так:

const config = [
	'region', 'district', 'area', 'city',
	{id:'place_id', name:'cityName'}, {id:'name', name:'streetName'},
];

clearAllControl() {
	config.forEach(x=>{
		let id = x.id || (x + '_id');
		let name = x.name || (x + 'Name');
		if (this.tableParam[id]) {
			delete this.tableParam[id];
			this.searchForm.get(name).patchValue(null);
		}
	});
	this.generateAddressesTable(this.tableParam);
}

Все названия говорящие. Всё понятно при чтении.
Насчет однотипности от ситуации зависит. Если из 10000 элементов 50 нуждаются в особом подходе, то нет смысла ради них расширять формат всех 10 тыс.

[TTSmaRt]

dollar,
...
x='place'
...
let id = x.id || (x + '_id')
// id == 'place_id'
let name = x.name || (x + 'Name')
// name == ' placeName'
// should be - 'cityName'.
...
update config or code.

[dollar]

TTSmaRt, думал, что это ошибка не у меня. Исправил.

[TTSmaRt]

MaxKorz, Практика понятно плохая. Тогда коммент не туда. Есть задача отрефакторить именно этот код. Что там вокруг никому не известно и на сколько все печально... Если это метод для работы с объектом api, Вы потребуете поменять ключи объектов с которыми работает api, а если api стороннее?
А для понятности и коммент написать можно если прочитать и понять сложно.

[TTSmaRt]

dollar, раз уж делаем красиво и круто, то и потеряшку вернуть надо )))
this.generateAddressesTable(this.tableParam);

[dollar]

TTSmaRt, Готово