Несколько слов о рефакторинге

Работая над большим проектом (100К+ строк кода) в разнонациональном коллективе (европейцы, индусы, американцы и японцы) рано или поздно доходишь до состояния «да я бы написал это куда лучше!», «надо это переписать», «да какой .. так пишет?!» и т.д. Если человек не ленивый, то у него такие мысли не расходятся с действиями. И я говорю не про жалобы руководству или обсуждение между коллегами, а про рефакторинг.

Рефа́кторинг (англ. refactoring) или реорганизация кода — процесс изменения внутренней структуры программы, не затрагивающий её внешнего поведения и имеющий целью облегчить понимание её работы.

Вообще, это довольно интересный процесс — ты берешь достаточно запутанный клубочек и распутываешь его до тех пор, пока его поведение не станет понятным даже новичку. Методы, которые при этом используются, отличаются как по своей сложности, так и по времени, необходимому для их внедрения. Однако, все они нуждаются в достаточном покрытии кода тестами. Иначе не избежать большого количества новых багов.

Хорошим теоретическим базисом для рефакторинга будет книга Мартина Фаулера «Рефакторинг. Улучшение существующего кода». Далее в этой заметке я буду использовать названия, предложенные в вышеуказанной книге.

Мартин Ф. выделил около 70 разных методов. Проанализировав свои действия, я понял, что использую от силы 10-15 из них (век живи — век учись). Далее будут изложены эти методы с кратким описанием и примерами на Ember JS.

В каждом примере
App — это Ember.Application.
App.ajax — это компонент, который позволяет делать запросы на сервер.
App.db — local storage.

Выделение метода

Довольно легкий и простой рефакторинг. Суть в том, что бы разбить большой метод на маленькие. Часто, когда метод большой, то в нем выполняются несколько действий из разных «логических» частей. Например, сложные подсчеты и отправка обработанных данных на сервер:

doSomething: function(a) {
	var d = a.getSome(), b = this.func(d);
	var f = Em.A([]);
	for (var i = 0; i < b; i++) {
		if (i % 2) {
			f.push(d);
		}
		else {
			f.push(this.calc(d));
		}
	}
	App.ajax.send('some_url', f);
}

Из этой функции будет вынесено все, кроме последней строчки:

calcSomething: function(a) {
	var d = a.getSome(), b = this.func(d);
	var f = Em.A([]);
	for (var i = 0; i < b; i++) {
		if (i % 2) {
			f.push(d);
		}
		else {
			f.push(this.calc(d));
		}
	}
	return f;
},
 
sendSomething: function(a) {
	App.ajax.send('some_url', this.calcSomething(a));
}

Тут стоит учесть, что подсчеты могут быть вынесены не просто в отдельный метод этого же объекта, а даже в отдельный модуль (это уже будет «Перемещение метода»).

Встраивание временной переменной

Тоже очень простой метод. Внутри какой-то функции есть переменная, которая инициализируется и больше с ней практически ничего не происходит. Такое часто встречается перед return’ом:

calcSomething: function(a) {
	var b = this.get('b');
	var f = b + a.get('c');
	return f;
}

Переменная f здесь не нужна. Ее можно убрать:

calcSomething: function(a) {
	var b = this.get('b');
	return b + a.get('c');
}

Вообще, хорошая IDE (использую PHP Storm) сама указывает на такие места в коде и рекомендует выполнить рефакторинг, но на это часто закрывают глаза. А в данном примере можно пойти дальше и убрать b:

calcSomething: function(a) {
	return this.get('b') + a.get('c');
}

«Читаемость» такой функции не ухудшилась ни на грамм.

Замещение алгоритма

Находим код, логика которого тяжела для восприятия сразу, и упрощаем его более простым вариантов. Или же попадается на руки образец «китайского» кода и убираем кучу однотипных элементов. Это куда более объемный метод. Но и пользы от него очень много.

installableClientComponents: function() {
	var installableClients = [];
	App.Service.find().forEach(function(svc) {
		switch(svc.get('serviceName')) {
			case 'PIG':
				installableClients.push('PIG');
				break;
			case 'SQOOP':
				installableClients.push('SQOOP');
				break;
			case 'HCATALOG':
				installableClients.push('HCAT');
				break;
			case 'HDFS':
				installableClients.push('HDFS_CLIENT');
				break;
		}
	});
	this.get('hostComponents').forEach(function (component) {
		var index = installableClients.indexOf(component.get('componentName'));
		if (index > -1) {
			installableClients.splice(index, 1);
		}
	}, this);
	return installableClients;
}.property()

Что этот код вообще пытается делать? В нашем приложении есть модель Service, Host и HostComponent. У каждого сервиса есть компоненты (хост-компоненты), которые устанавливаются на хосты. Каждый компонент может быть master, slave или client. Нам надо получить массив client хост-компонентов, которых еще нет на хосте (в контексте приведенного фрагмента кода this — это хост). В данном фрагменте программы «захардкожено» соотношение Сервис-Компонент (switch-case). Это надо убрать. Так же надо знать, что в системе уже есть map, где указанно соотношения компонент-сервис. И еще одно — если какого-то сервиса в системе нет, то его client не может быть установлен (а значит, не может попасть в наш список).

installableClientComponents: function() {
	var componentServiceMap = App.QuickDataMapper.componentServiceMap(); // Та самая map. Ключ - имя компонента, значение - имя сервиса
	var allClients = App.get('components.clients'); // Список всех компонентов, которые client
	var installedServices = this.get('installedServices'); // Список имен установленных сервисов
	var installedClients = this.get('clients'); // Список уже установленных clients
	return allClients.filter(function(componentName) {
		// service for current client is installed but client isn't installed on current host
		return installedServices.contains(componentServiceMap[componentName]) && !installedClients.contains(componentName);
	});
}.property()

Может показаться, что мы просто перенесли map’у из одного места в другое, но это не так. Во-первых, map уже была написана до написания вышеуказанного кода. Во-вторых, она активно используется в других модулях системы. В-третьих, поменялся алгоритм формирования списка. В первом случае мы набирали список всех возможных клиентов (с установленных сервисов) и потом отбрасывали из него уже установленные (при чем проганяли по всем компонентам — не только по client). Во втором варианте мы проходим по списку всех клиентов и убираем из него недоступные и уже установленные компоненты.

По своей сути нового ничего не происходит (что есть нормально, ведь мы делаем только рефакторинг), но читабельность кода возросла, а так же теперь не надо при появлении нового сервиса в системе (у которого есть client-компонент) бегать по нескольким местам в коде и добавлять еще один case в switch или if-else-блок (потому что таких блоков нет 🙂 ).

Замена магического числа символической константой

Читать буквы/слова куда проще, чем цифры/числа:

doSomething: function(a) {
	App.ajax.send(a.get('value')).retry({times: 4, timeout: 30000});
}
 
doAnother: function(a) {
	App.ajax.send({val: a.get('value'), name: a.get('propertyName')}).retry({times: 4, timeout: 30000});
}

Из кода понятно, что такое 4 и 30000. Но как быть, если внезапно захочется вместо 4 попыток делать 3 или 5? Придется бегать по всему коду и делать замену. Куда лучше вынести их в отдельные константы в App или App.ajax:

var App = Ember.Application.create({
	/*.....*/
	retryTimes: 4,
	timeout: 30000
	/*.....*/
});
/*........................*/
doSomething: function(a) {
	App.ajax.send(a.get('value')).retry({times: App.retryTimes, timeout: App.timeout});
}
 
doAnother: function(a) {
	App.ajax.send({val: a.get('value'), name: a.get('propertyName')}).retry({times: App.retryTimes, timeout: App.timeout});
}

Декомпозиция условного оператора

В ситуациях, когда в какой-то функции возникает очень большое условное выражение (тут может быть как сложность самого условия, так и сложность веток if-else). Пример сложного условия:

/**
 *
 * @param {App.A} a
 */
calcSomething: function(a) {
	var b = this.get('b'), c;
	if (this.get('isActive') && a.get('field') > 0 && a.get('field') < a.get('data').length) {
		c = this.update1(a.get('field')) + this.get('c') - b;
	}
	else {
		c = this.update2(a.get('field')) + b;
	}
	return c;
}
 
/*******************************/
App.A = Em.Object.extend({
	field: 0,
	data: []
})

Условное выражение лучше вынести как свойство объекта а:

/**
 * @param {App.A} a
 */
calcSomething: function(a) {
	var b = this.get('b'), c;
	if (this.get('isActive') && a.get('readyForCalc')) {
		c = this.update1(a.get('field')) + this.get('c') - b;
	}
	else {
		c = this.update2(a.get('field')) + b;
	}
	return c;
}
 
/*******************************/
App.A = Em.Object.extend({
	field: 0,
	data: [],
	readyForCalc: function() {
		return this.get('field') > 0 && this.get('field') < this.get('data').length;
	}.property('isActive', 'field', 'data.length')
})

Пример сложных веток if-else:

calcSomething: function(storedCfgs, preDefinedCfgs) {
	if (storedCfgs.length <= 1 && preDefinedCfgs.length <= 1) {
		var stored = storedCfgs[0];
		var preDefined = preDefinedCfgs[0];
		configData = preDefined;
		configData.value = stored.value;
		configData.defaultValue = stored.defaultValue;
		configData.overrides = stored.overrides;
		configData.filename = stored.filename;
		configData.description = stored.description;
		configData.isRequiredByAgent = (configData.isRequiredByAgent !== undefined) ? configData.isRequiredByAgent : true;
		configData.showLabel = stored.showLabel !== false;
		/* Еще 40 строк кода */
	}
	else {
		preDefinedCfgs.forEach(function (cfg) {
			configData = cfg;
			configData.isRequiredByAgent = (configData.isRequiredByAgent !== undefined) ? configData.isRequiredByAgent : true;
			var storedCfg = storedCfgs.findProperty('filename', cfg.filename);
			if (storedCfg) {
				configData.value = storedCfg.value;
				configData.defaultValue = storedCfg.defaultValue;
				configData.overrides = storedCfg.overrides;
				configData.filename = storedCfg.filename;
				configData.description = storedCfg.description;
				configData.description = storedCfg.showLabel !== false;
			} else if (isAdvanced){
				advanced = advancedConfigs.filterProperty('filename', configData.filename).findProperty('name', configData.name);
				this.setPropertyFromStack(configData,advanced);
			}
			mergedConfigs.push(configData);
		}, this);
		/* Еще строк 25-30 кода */
	}
}

Логику работы такой функции тяжело держать в голове. Необходимо вынести ветки if, else в отдельные функции с нормальными именами:

calcSomething: function(storedCfgs, preDefinedCfgs) {
	if (storedCfgs.length <= 1 && preDefinedCfgs.length <= 1) {
		this.procOneConfig(storedCfgs[0], preDefinedCfgs[0]);
	}
	else {
		this.procManyConfigs(storedCfgs, preDefinedCfgs);
	}
}

В данном примере имена получились не самые понятные, но хотя бы ясно, с какими множествами работают новые методы. Да и тестировать несколько небольших функций куда проще, чем одну большую.

Консолидация дублирующих условных фрагментов

Если в ветках if-else есть одинаковые блоки кода, то их необходимо вынести и поставить после условного оператора.

doSomething: function() {
	var value = this.get('value'), defaultValue = this.get('defaultValue'), newValue;
	if (Ember.compare(val, defaultValue) == -1) {
		newValue = this.permutate(val);
		App.ajax.send(newValue);
	}
	else {
		newValue = defaultValue;
		App.ajax.send(newValue);
	}
}

Вызов App.ajax.send есть в двух ветках. Вынесем его вниз:

doSomething: function() {
	var value = this.get('value'), defaultValue = this.get('defaultValue'), newValue;
	if (Ember.compare(val, defaultValue) == -1) {
		newValue = this.permutate(val);
	}
	else {
		newValue = defaultValue;
	}
	App.ajax.send(newValue);
}

В этом примере кода стало меньше всего на одну строку, но в некоторых ситуациях возможно, что из веток if, else вниз будет вынесена бОльшая часть их кода (а в них останется только условиезависимая инициализация некоторых локальных переменных). Да, вынос может быть и перед условным оператором (зависит от расположения дублирующих фрагментов в ветках if-else).

Замена условного оператора полиморфизмом

Применяется в случаях, когда в методе родительского класса есть код, вариант выполнения которого зависит от типа класса-потомка. Необходимо логику для каждого из потомков вынести в дочерние классы. Пример кода с таким условным оператором:

App.WizardController = Em.Controller.extend({
	/*********/
	installServices: function (isRetry) {
		var data, name;
 
		switch (this.get('content.controllerName')) {
			case 'addHostController':
				if (isRetry) {
					name = 'wizard.install_services.add_host_controller.is_retry';
				}
				else {
					name = 'wizard.install_services.add_host_controller.not_is_retry';
				}
				data = {
					"RequestInfo": {
						"context": Em.I18n.t('requestInfo.installComponents'),
						"query": "HostRoles/host_name.in(" + App.db.getHosts().join(',') + ")"
					},
					"Body": {
						"HostRoles": {"state": "INSTALLED"}
					}
				};
				data = JSON.stringify(data);
				break;
 
			case 'addServiceController':
				if (isRetry) {
					this.getFailedHostComponents();
					name = 'wizard.install_services.installer_controller.is_retry';
					data = {
						"RequestInfo": {
							"context" : "installComponents",
							"query": "HostRoles/component_name.in(" + this.get('failedHostComponents').join(',') + ")"
						},
						"Body": {
							"HostRoles": {
								"state": "INSTALLED"
							}
						}
					};
					data = JSON.stringify(data);
				}
				else {
					name = 'wizard.install_services.installer_controller.not_is_retry';
					data = '{"RequestInfo": {"context" :"installServices"}, "Body": {"ServiceInfo": {"state": "INSTALLED"}}}';
				}
				break;
 
			case 'installerController':
			default:
				if (isRetry) {
					name = 'wizard.install_services.installer_controller.is_retry';
					data = '{"RequestInfo": {"context" :"installComponents"}, "Body": {"HostRoles": {"state": "INSTALLED"}}}';
				}
				else {
					name = 'wizard.install_services.installer_controller.not_is_retry';
					data = '{"RequestInfo": {"context" :"installServices"}, "Body": {"ServiceInfo": {"state": "INSTALLED"}}}';
				}
				break;
		}
 
		App.ajax.send({
			name: name,
			sender: this,
			data: {
				data: data
			},
			success: 'installServicesSuccessCallback',
			error: 'installServicesErrorCallback'
		});
	}
	/*********/
});

Метод installServices в зависимости от контроллера, в котором вызывается, передает соответствующие параметры в ajax-запрос. Получается, что родитель знает про своих потомков, а это недопустимо. Надо разбить его на части и поместить каждую в свой контроллер. AddHostController:

App.AddHostController = App.WizardController.extend({
 
	name: 'addHostController',
 
	/*********/
	installServices: function (isRetry) {
		var data, name;
 
		if (isRetry) {
			name = 'wizard.install_services.add_host_controller.is_retry';
		}
		else {
			name = 'wizard.install_services.add_host_controller.not_is_retry';
		}
		data = {
			"RequestInfo": {
				"context": "installComponents",
				"query": "HostRoles/host_name.in(" + App.db.getHosts().join(',') + ")"
			},
			"Body": {
				"HostRoles": {"state": "INSTALLED"}
			}
		};
		data = JSON.stringify(data);
		App.ajax.send({
			name: name,
			sender: this,
			data: {
				data: data
			},
			success: 'installServicesSuccessCallback',
			error: 'installServicesErrorCallback'
		});
	}
	/*********/
});

AddServiceController:

App.AddServiceController = App.WizardController.extend({
 
	name: 'addServiceController',
 
	/*********/
	installServices: function (isRetry) {
		var data, name;
		if (isRetry) {
			this.getFailedHostComponents();
			name = 'wizard.install_services.installer_controller.is_retry';
			data = {
				"RequestInfo": {
					"context" : "installComponents",
					"query": "HostRoles/component_name.in(" + this.get('failedHostComponents').join(',') + ")"
				},
				"Body": {
					"HostRoles": {
						"state": "INSTALLED"
					}
				}
			};
			data = JSON.stringify(data);
		}
		else {
			name = 'wizard.install_services.installer_controller.not_is_retry';
			data = '{"RequestInfo": {"context" :"installServices"}, "Body": {"ServiceInfo": {"state": "INSTALLED"}}}';
		}
		App.ajax.send({
			name: name,
			sender: this,
			data: {
				data: data
			},
			success: 'installServicesSuccessCallback',
			error: 'installServicesErrorCallback'
		});
	}
	/*********/
});

В App.WizardController останется только та часть, которая в ветке default:

App.WizardController = Em.Controller.extend({
 
	/*********/
	installServices: function (isRetry) {
		var data, name;
		if (isRetry) {
			name = 'wizard.install_services.installer_controller.is_retry';
			data = '{"RequestInfo": {"context" :"installComponents"}, "Body": {"HostRoles": {"state": "INSTALLED"}}}';
		}
		else {
			name = 'wizard.install_services.installer_controller.not_is_retry';
			data = '{"RequestInfo": {"context" :"installServices"}, "Body": {"ServiceInfo": {"state": "INSTALLED"}}}';
		}
 
		App.ajax.send({
			name: name,
			sender: this,
			data: {
				data: data
			},
			success: 'installServicesSuccessCallback',
			error: 'installServicesErrorCallback'
		});
	}
	/*********/
});

В данном примере рефакторинг можно продолжить и вынести вызов App.ajax.send в отдельный метод, который будет принимать два параметра:

sendRequest: function(name, data) {
	App.ajax.send({
		name: name,
		sender: this,
		data: {
			data: data
		},
		success: 'installServicesSuccessCallback',
		error: 'installServicesErrorCallback'
	});
}

Но это уже на личное усмотрение. Здесь основным было разнести логику по нужным объектам.

Переименовывание метода

Имя функции должно отражать совершаемое ею действие (иными словами содержать глагол). Пример не удачного названия:

service: function() {
	var services = App.ajax.load('services').mapProperty('serviceName').without('HIVE');
	this.set('services', services);
}

Такое имя функции вообще не отражает ее суть и больше подходит для имени поля. Переименуем в:

loadServices: function() {
	var services = App.ajax.load('services').mapProperty('serviceName').without('HIVE');
	this.set('services', services);
}

Так куда лучше. Вопрос о том, стоит ли тут делать «Встраивание временной переменной» остается для личного обдумывания читателем.

Сокрытие метода

«Из коробки» понятий public, private в JS нет. Приходится выкручиваться. Пример кода, где метод formatData доступен извне:

App.V = Em.Mixin.create({
 
	/**
	 * @type {String}
	 */
	data: '',
 
	formatData: function(data) {
		return data.toUpperCase().replace(/_/g, '-');
	}	
 
});

А теперь спрячем этом метод от посторонних:

(function() {
 
	formatData: function(data) { return data.toUpperCase().replace(/_/g, '-'); }
 
	App.V = Em.Mixin.create({
 
		/**
		 * @type {String}
		 */
		data: ''
 
	});
})();

Такие штуки хорошо реализуются в brunch.io. Каждый отдельный файл в нем при сборке всего проекта попадает в оболочку вида:

window.require.register("%NAME%", function(exports, require, module) {
	/* Код модуля */
});

То есть, если «миксина» («примесь», если угодно) App.V будет лежать в отдельном файле, то все, что будет написано за пределами App.V = Em.Mixin.create({…}) этого файла автоматически будет «private» для App.V и недоступно никому извне.

Подъем поля

Какое-то поле есть в подклассах одного класса. СтОит вынести его в родительский класс.

App.Service = DS.Model.extend({
	serviceName: DS.attr('string'),
	passiveState: DS.attr('string')
});
 
App.HBaseService = App.Service.extend({
	workStatus: DS.attr('string'),
	masterStartTime: DS.attr('number'),
	masterActiveTime: DS.attr('number')
});
 
App.HDFSService = App.Service.extend({
	workStatus: DS.attr('string'),
	capacityUsed: DS.attr('number'),
	capacityTotal: DS.attr('number')
});

Поле workStatus объявлено в двух дочерних моделях. Его нужно вынести в App.Service:

App.Service = DS.Model.extend({
	workStatus: DS.attr('string'),
	serviceName: DS.attr('string'),
	passiveState: DS.attr('string')
});

Подъем метода

Тоже самое, что и «Подъем поля», но выносится уже целый метод:

App.WidgetView = Em.View.extend({
	data: null
});
 
App.ChartWidgetView = App.WidgetView.extend({
	loadData: function() {
		this.set('data', App.ajax.load('chart1'));
	}
});
 
App.TextWidgetView = App.WidgetView.extend({
	loadData: function() {
		this.set('data', App.ajax.load('text1'));
	}
});

Метод loadData выполняет одно и тоже в двух классах виджетов и должен быть вынесен в App.WidgetView:

App.WidgetView = Em.View.extend({
	data: null,
	field: null,
	loadData: function() {
		var field = this.get('field');
		this.set('data', App.ajax.load(field));
	}
});
 
App.ChartWidgetView = App.WidgetView.extend({
	field: 'chart1'
});
 
App.TextWidgetView = App.WidgetView.extend({
	field: 'text1'
});

Выделение подкласса (примеси)

В ситуациях, когда часть функционала класса используется только в некоторых случаях. Как пример рассмотрим ситацию, когда есть приложение со списком хост-компонентов (см. «Замещение алгоритма»). Так же есть класс view для каждого компонента:

App.HostComponentView = Em.View.extend({
 
	templateName: require('templates/main/host/details/host_component'),
 
	/**
	 * @type {App.HostComponent}
	 */
	content: null,
 
	/**
	 * No action available while component is starting/stopping/unknown
	 * @type {String}
	 */
	noActionAvailable: function () {
		var workStatus = this.get('workStatus');
		if ([App.HostComponentStatus.starting, App.HostComponentStatus.stopping, App.HostComponentStatus.unknown].contains(workStatus)) {
			return "hidden";
		}else{
			return "";
		}
	}.property('workStatus'),
 
	didInsertElement: function () {
		App.tooltip($('[rel=componentHealthTooltip]'));
		App.tooltip($('[rel=passiveTooltip]'));
	},
 
	/**
	 * Received from server object with data about decommission
	 * @type {Object}
	 */
	decommissionedStatusObject: null,
 
	/**
	 * Is component in decommission process right know
	 * @type {bool}
	 */
	isComponentDecommissioning: null,
 
	/**
	 * Get component decommission status from server
	 * @returns {$.ajax}
	 */
	getDecommissionStatus: function() {
		return App.ajax.send({
			name: 'host.host_component.decommission_status',
			sender: this,
			data: {
				hostName: this.get('content.host.hostName'),
				serviceName: this.get('content.service.serviceName')
			},
			success: 'getDecommissionStatusSuccessCallback',
			error: 'getDecommissionStatusErrorCallback'
		});
	},
 
	/**
	 * Set received value or null to <code>decommissionedStatusObject</code>
	 * @param {Object} response
	 * @returns {Object|null}
	 */
	getDecommissionStatusSuccessCallback: function (response) {
		var statusObject = response.ServiceComponentInfo;
		if ( statusObject != null) {
			this.set('decommissionedStatusObject', statusObject);
			return statusObject;
		}
	},
 
	/**
	 * Set null to <code>decommissionedStatusObject</code> if server returns error
	 * @returns {null}
	 */
	getDecommissionStatusErrorCallback: function () {
		this.set('decommissionedStatusObject', null);
	}
});

Известно, что «Decommission» доступно только для части компонентов. Значит, в «общей» view некоторых методам/свойствам быть не должно. Это:

  • decommissionedStatusObject
  • isComponentDecommissioning
  • getDecommissionStatus
  • getDecommissionStatusSuccessCallback
  • getDecommissionStatusErrorCallback

Есть два варианта действий — сделать дочерний класс или примесь (mixin). Какой из вариантов лучше — не могу ответить. Оба упрощают код и не вызывают особых трудностей в реализации. Сделаем mixin:

App.Decommissionable = Em.Mixin.create({
 
	/**
	 * Received from server object with data about decommission
	 * @type {Object}
	 */
	decommissionedStatusObject: null,
 
	/**
	 * Is component in decommission process right know
	 * @type {bool}
	 */
	isComponentDecommissioning: null,
 
	/**
	 * Get component decommission status from server
	 * @returns {$.ajax}
	 */
	getDecommissionStatus: function() {
		return App.ajax.send({
			name: 'host.host_component.decommission_status',
			sender: this,
			data: {
				hostName: this.get('content.host.hostName'),
				serviceName: this.get('content.service.serviceName')
			},
			success: 'getDecommissionStatusSuccessCallback',
			error: 'getDecommissionStatusErrorCallback'
		});
	},
 
	/**
	 * Set received value or null to <code>decommissionedStatusObject</code>
	 * @param {Object} response
	 * @returns {Object|null}
	 */
	getDecommissionStatusSuccessCallback: function (response) {
		var statusObject = response.ServiceComponentInfo;
		if ( statusObject != null) {
			this.set('decommissionedStatusObject', statusObject);
			return statusObject;
		}
	},
 
	/**
	 * Set null to <code>decommissionedStatusObject</code> if server returns error
	 * @returns {null}
	 */
	getDecommissionStatusErrorCallback: function () {
		this.set('decommissionedStatusObject', null);
	}
});

В App.HostComponentView останется только:

App.HostComponentView = Em.View.extend({
 
	templateName: require('templates/main/host/details/host_component'),
 
	/**
	 * @type {App.HostComponent}
	 */
	content: null,
 
	/**
	 * No action available while component is starting/stopping/unknown
	 * @type {String}
	 */
	noActionAvailable: function () {
		var workStatus = this.get('workStatus');
		if ([App.HostComponentStatus.starting, App.HostComponentStatus.stopping, App.HostComponentStatus.unknown].contains(workStatus)) {
			return "hidden";
		}else{
			return "";
		}
	}.property('workStatus'),
 
	didInsertElement: function () {
		App.tooltip($('[rel=componentHealthTooltip]'));
		App.tooltip($('[rel=passiveTooltip]'));
	}
});

Теперь, что бы создать view для хост-компонента с decommission надо использовать код вида:

App.SomeHostComponentView = App.HostComponentView.extend(App.Decommissionable, {
	/* Ваш код */
});

Выделение дочернего класса (или создание «примеси») является не самым простым рефакторингом, но позволяет получить большой профит в будущем. На счет mixin надо сказать дополнительно. Вполне может быть ситуация, когда общие фрагменты будут находиться в объектах, которые совсем не связанны между собой. К примеру, в приложении есть возможность сохранять некоторые настройки UI на сервере. Реализовано это самым простым POST-запросом «ключ=значение» (одна пара за один запрос). Чтение настроек делается через GET по ключу (так же по одной за один запрос). Список настроек:

  • Количество выводимых записей в таблице компонентов
  • Автозакрытие popup’ов

Очевидно, что это все «разноуровневые» данные, которые лежат в разных контроллерах (или видах). И изначально у каждого из них есть свой метод для post и get запросов для работы с настройками:

App.ApplicationController = Em.Controller.extend({
	/**
	 * get persist value from server with persistKey
	 */
	getUserPref: function(key){
		return App.ajax.send({
			name: 'settings.get.user_pref',
			sender: this,
			data: {
				key: key
			},
			success: 'getUserPrefSuccessCallback',
			error: 'getUserPrefErrorCallback'
		});
	},
	getUserPrefSuccessCallback: function (response) {
		this.set('currentPrefObject', response);
		return response;
	},
	getUserPrefErrorCallback: function (request, ajaxOptions, error) {
			this.set('currentPrefObject', true);
			this.postUserPref(this.persistKey(), true);
			return true;
		}
	},
	/**
	 * post persist key/value to server, value is object
	 */
	postUserPref: function (key, value) {
		var keyValuePair = {};
		keyValuePair[key] = JSON.stringify(value);
		App.ajax.send({
			'name': 'settings.post.user_pref',
			'sender': this,
			'data': {
				'keyValuePair': keyValuePair
			}
		});
	}
});
 
App.TableView = Em.View.extend({
 
	 /**
	 * get display length persist value from server with displayLengthKey
	 */
	getUserPref: function(key){
		return App.ajax.send({
			name: 'settings.get.user_pref',
			sender: this,
			data: {
				key: key
			},
			success: 'getDisplayLengthSuccessCallback',
			error: 'getDisplayLengthErrorCallback'
		});
	},
 
	/**
	 * Set received from server value to <code>displayLengthOnLoad</code>
	 * @param {Number} response
	 * @param {Object} request
	 * @param {Object} data
	 * @returns {*}
	 */
	getDisplayLengthSuccessCallback: function (response, request, data) {
		this.set('displayLengthOnLoad', response);
		return response;
	},
 
	/**
	 * Set default value to <code>displayLengthOnLoad</code> (and send it on server) if value wasn't found on server
	 * @returns {Number}
	 */
	getDisplayLengthErrorCallback: function () {
		var displayLengthDefault = "10";
		this.set('displayLengthOnLoad', displayLengthDefault);
		if (App.get('isAdmin')) {
			this.postUserPref(this.displayLengthKey(), displayLengthDefault);
		}
		return displayLengthDefault;
	},
 
	/**
	 * Post display length persist key/value to server
	 * @param {String} key
	 * @param {Object} value
	 */
	postUserPref: function (key, value) {
		var keyValuePair = {};
		keyValuePair[key] = JSON.stringify(value);
		App.ajax.send({
			name: 'settings.post.user_pref',
			sender: this,
			data: {
				keyValuePair: keyValuePair
			}
		});
	}
});

Что контроллер, что вид содержат и другие методы и свойства, но я указал только те, которые нужны в данном примере. Как видно, отличаются они только методами обратного вызова (callback’ами) getUserPref. Смело выносим остальное в примесь:

/**
 * Small mixin for processing user preferences
 * Provide methods to save/load some values in <code>persist</code> storage
 * When using this mixin you should redeclare methods:
 * <ul>
 *   <li>getUserPrefSuccessCallback</li>
 *   <li>getUserPrefErrorCallback</li>
 * </ul>
 * @type {Em.Mixin}
 */
App.UserPref = Em.Mixin.create({
  /**
   * Get persist value from server with persistKey
   * @param {String} key
   */
  getUserPref: function(key) {
    return App.ajax.send({
      name: 'settings.get.user_pref',
      sender: this,
      data: {
        key: key
      },
      success: 'getUserPrefSuccessCallback',
      error: 'getUserPrefErrorCallback'
    });
  },
 
  /**
   * Should be redeclared in objects that use this mixin
   * @param {*} response
   * @param {Object} request
   * @param {Object} data
   * @returns {*}
   */
  getUserPrefSuccessCallback: function (response, request, data) {},
 
  /**
   * Should be redeclared in objects that use this mixin
   * @param {Object} request
   * @param {Object} ajaxOptions
   * @param {String} error
   */
  getUserPrefErrorCallback: function (request, ajaxOptions, error) {},
 
  /**
   * Post persist key/value to server, value is object
   * @param {String} key
   * @param {Object} value
   */
  postUserPref: function (key, value) {
    var keyValuePair = {};
    keyValuePair[key] = JSON.stringify(value);
    App.ajax.send({
      'name': 'settings.post.user_pref',
      'sender': this,
      'data': {
        'keyValuePair': keyValuePair
      }
    });
  }
 
});

Теперь объявление контроллера и вида будет таким:

App.ApplicationController = Em.Controller.extend(App.UserPref, {
	getUserPrefSuccessCallback: function() { /*Code*/ },	
	getUserPrefErrorCallback: function() { /*Code*/ }
});
 
App.TableView = Em.View.extend(App.UserPref, {
	getUserPrefSuccessCallback: function() { /*Code*/ },
	getUserPrefErrorCallback: function() { /*Code*/ }
});

Понятно, что о выделении тут какого-то родительского класса не могло быть и речи, а вот создание примеси вписалось весьма удачно. Не будь в Ember примесей пришлось бы создавать отдельный «helper», но это было бы куда более громоздким решением.

Вместо P.S.

Никакой рефакторинг не возможен, если код не покрыт тестами. Без тестов больше шансов добавить новых багов в код, нежели улучшить его читаемость и упростить сопровождение.

,

Оставить комментарий

Top ↑ | Main page | Back