iproger
@iproger
Безответственный гений

Как стать на правильный путь в написании кода?

Я пишу код уже несколько лет, но последнее время меня начало уже даже не раздражать, а бесить качество своего кода. Самое обидное, когда ты даже не знаешь, как сделать лучше.
Например, модуль авиаперелетов (JS):
for (var i=0;i<len;i++) {
		if (data.Flight.hasOwnProperty(i)) {
			var segm_data = data.Flight[i].Segment;
			var suppliers = get_supplier(segm_data);
			//console.log(segm_data);return true;
			var flight_start_time = segm_data[0].Departure.attributes.Time;
			var flight_end_time = segm_data[0].Arrival.attributes.Time;
			var departure = segm_data[0].Departure.attributes;
			var arrival = segm_data[0].Arrival.attributes;
			//console.log(arrival);
			var dep_all_time = arrival.Date.split('-').reverse().join('-')+' '+arrival.Time;
			var arr_all_time = departure.Date.split('-').reverse().join('-')+' '+departure.Time;
			var flight_time = moment(dep_all_time).diff(arr_all_time,'seconds');
			if (
				(time2minutes(flight_start_time) < parseInt(this.filter.data.time.departure.start))
				|| (time2minutes(flight_start_time) > parseInt(this.filter.data.time.departure.end))
				|| (time2minutes(flight_end_time) < parseInt(this.filter.data.time.departure_back.start))
				|| (time2minutes(flight_end_time) > parseInt(this.filter.data.time.departure_back.end))
				|| flight_time > parseInt(this.filter.data.time.flight)*60
				|| !in_array_array(suppliers,this.filter.data.airlines)
			) {
				status = false;
				break;
			}
		}
	}


Это главный цикл, который отвечает за "попадание" элемента (перелета) в список на странице. Я вижу как одно из решений - бить все на мелкие функции, но хотелось бы услышать совета проф.
  • Вопрос задан
  • 2740 просмотров
Пригласить эксперта
Ответы на вопрос 6
@artishok
кратко
Я бы посоветовал оптимизировать строки
var flight_start_time = segm_data[0].Departure.attributes.Time;
var flight_end_time = segm_data[0].Arrival.attributes.Time;
var departure = segm_data[0].Departure.attributes;
var arrival = segm_data[0].Arrival.attributes;
var dep_all_time = arrival.Date.split('-').reverse().join('-')+' '+arrival.Time;
var arr_all_time = departure.Date.split('-').reverse().join('-')+' '+departure.Time;

заменив на
var departure = segm_data[0].Departure.attributes;
var arrival = segm_data[0].Arrival.attributes;
var flight_start_time = departure.Time;
var flight_end_time = arrival.Time;            
var dep_all_time = arrival.Date.split('-').reverse().join('-')+' '+flight_end_time;
var arr_all_time = departure.Date.split('-').reverse().join('-')+' '+flight_start_time;
Ответ написан
Комментировать
BuriK666
@BuriK666
Компьютерный псих
У вас 4 раза используется segm_data[0], и нигде не используется просто segm_data
может тогда лучше сделать так:
var segm_data = data.Flight[i].Segment[0];
аналогично с flight_start_time и flight_end_time, к ним везде применяется time2minutes()
var flight_start_time = time2minutes(segm_data.Departure.attributes.Time);

А на какие маленькие функции вы бы хотели разбить этот кусок?
Ответ написан
Комментировать
alekciy
@alekciy
Вёбных дел мастер
Вот меня смущает обилие var. Без такого количества дополнительных переменных реально ни как не обойтись? Учитывая отсутствие контекста другого кода бы рекомендовал для начала хорошо форматировать код. Например, так:

for (var i = 0; i < len; ++i)
{
	if ( !data.Flight.hasOwnProperty(i) ) {
		continue;
	}

	var
		  segm_data         = data.Flight[i].Segment
		, suppliers         = get_supplier(segm_data)
		, flight_start_time = segm_data[0].Departure.attributes.Time
		, flight_end_time   = segm_data[0].Arrival.attributes.Time
		, departure         = segm_data[0].Departure.attributes
		, arrival           = segm_data[0].Arrival.attributes
	;

	var
		  dep_all_time = arrival.Date.split('-').reverse().join('-')+' '+arrival.Time
		, arr_all_time = departure.Date.split('-').reverse().join('-')+' '+departure.Time
		, flight_time  = moment(dep_all_time).diff(arr_all_time,'seconds')
	;
	
	if (
		(time2minutes(flight_start_time) < parseInt(this.filter.data.time.departure.start))
		|| (time2minutes(flight_start_time) > parseInt(this.filter.data.time.departure.end))
		|| (time2minutes(flight_end_time) < parseInt(this.filter.data.time.departure_back.start))
		|| (time2minutes(flight_end_time) > parseInt(this.filter.data.time.departure_back.end))
		|| flight_time > parseInt(this.filter.data.time.flight)*60
		|| !in_array_array(suppliers,this.filter.data.airlines)
	) {
		status = false;
		break;
	}

}
Ответ написан
Комментировать
Anonym
@Anonym
Программирую немного )
Уменьшайте вложенность.
// Было
for (var i=0;i<len;i++) {
    if (data.Flight.hasOwnProperty(i)) {
        // ...
    }
}
// Стало
for (var i=0;i<len;i++) if (data.Flight.hasOwnProperty(i)) {
    // ...
}

// Было
for (var i=0;i<flights.search_result.Air.length;i++) {
    if (flights.search_result.Air.hasOwnProperty(i)) {
        // ...
    }
}
// Стало
for (var i=0;i<flights.search_result.Air.length;i++) {
    if (!flights.search_result.Air.hasOwnProperty(i)) continue;
    // ...
}
Ответ написан
parmactep
@parmactep
Как-то много сущностей вы плодите. Может есть смысл их заворачивать в обьекты...
Исходя из количеству циклов могу посоветовять нечто наподобие each из underscore.js
Ответ написан
Комментировать
Anonym
@Anonym
Программирую немного )
Вот вам еще оптимизация. Посмотрите на этот код (у вас много подобного):
for (var i=0; i<arr.length; i++) {
    if (!arr.hasOwnProperty(i)) continue;
    // ...
}

Во-первых, цикл. При обходе массива лучше делать так, будет быстрее:
for (var i=0, l=arr.length; i<l; i++) {
    // ...
}

Или так:
var l = arr.length;
while (l--) {
    // ...
}

Во-вторых, зачем вы проверяете arr.hasOwnProperty(i)? Вы обходите массив последовательно от 0 до arr.length-1, то есть если индексы в массиве от 0 до arr.length-1, то можно не проверять, вы обойдете все элементы, а если индексы проставлены иначе, то вы обойдете не весь массив и ваш код не работает как надо. Такую проверку обычно делают при обходе объекта с помощью for ... in ...
Ответ написан
Комментировать
Ваш ответ на вопрос

Войдите, чтобы написать ответ

Войти через центр авторизации
Похожие вопросы