Не говнокод ли я пишу?

Изучаю javascript и не знаю, хороший ли код я пишу в архитектурном плане, хоть и с паттернами знаком.
plnkr.co/edit/tftlBb9toYbIbBRULmrE?p=preview
Кто что может подсказать? Как можно улучшить эту простыню?
  • Вопрос задан
  • 1499 просмотров
Решения вопроса 1
Deerenaros
@Deerenaros
Программист, математик, задрот и даже чуть инженер
Есть jquery. Сто лет ему. Немного (сильно) сокращает код.
this.onMouseMoveBinded = this.onMouseMove.bind(this);
this.onMouseEndBinded = this.onMouseEnd.bind(this);


Не прочитать.
if( typeof (this.obj) === 'undefined' ) this.obj = opt.el;

Лучше так.
if(this.obj === undefined) {
	this.obj = opt.el;
}

Вам срочно нужна векторная алгебра.
// ***
	this.shiftX = e.clientX - thumbCoords.left;
	this.shiftY = e.clientY - thumbCoords.top;
//***
	this.coords = {
		top: e.clientY - this.shiftY - this.parentShift.top,
		left: e.clientX - this.shiftX - this.parentShift.left
	};
//***
	this.coords.top = e.clientY - this.shiftY - this.parentShift.top;
	this.coords.left = e.clientX - this.shiftX - this.parentShift.left;
//***
	this.rx = this.px - this.kx;
	this.ry = this.py - this.ky;
//***

Ну jquery же.
doc.documentElement.classList.remove('grabbing');
doc.removeEventListener('mousemove', this.onMouseMoveBinded);
doc.removeEventListener('mouseup', this.onMouseEndBinded);

Вообще, подумайте над названиями. Может оно и адекватно, но мне - непонятно. Откуда тут напряжение взялось?
function Tension(opt){
	dragNdrop.apply(this, arguments);
}

Не помню как сейчас, но, ЕМНИП, в js объекты-классы определяются через var.
Tension.prototype = Object.create(dragNdrop.prototype);
Tension.prototype.constructor = Tension;

То есть декларирование Tension должно быть каким-то таким:
var Tension = function (opt){
	dragNdrop.apply(this, arguments);
}

Снова не прочитать условия. Не жалейте строк! Алсо, всё в одну кучу - UI, логику, физику - это плохой знак.
function Slider(opt){
	if(typeof (this.obj) === 'undefined') this.obj = opt.thump;
	if(typeof (this.slider) === 'undefined') this.slider = opt.slider;
	dragNdrop.apply(this, arguments);
}


new dragNdrop({
	el: doc.getElementById('ball1'),
	physiq: true
});

new dragNdrop({
	el: doc.getElementById('ball2')
});

new Tension({
	el: doc.getElementById('square1')
});

new Slider({
	slider: doc.getElementById('slider'),
	thump:  doc.querySelector('.thumb'),
	physiq: true
});

new StepSlider({
	el: doc.getElementById('slider-step'),
	from: 5,
	to: 40
});

Плачу кровавыми слезами. Что это? Не боитесь сборщика мусора? Не надо так. Вообще global space - не есть хорошо.

Итого мы имеем сложно читаемую математику, паршивый нейминг, неудобные условия и нежелание следовать каким-то best-practice. Это всё не фатально, более того - прототип рабочий. Меняться вам или нет - решайте сами. Из важного, я бы посоветовал сделать всё читаемым, а для этого потребуется адекватный нейминг, не сваливать всё в кучу и следовать неформальным правилам. В остальном, javascript не идеальный вариант, чтобы показывать задротство в области clear code. И с паттернами у него беда.
Ответ написан
Пригласить эксперта
Ответы на вопрос 3
@huwesu
Он родимый.
Но не самого крайнего пошиба.
Не корите себя.
Ответ написан
Комментировать
this.physik = opt.physiq
Проиграл на этом, попытки осмыслить бросил.
Ну правда, хотя бы по файлам разбейте что ли.
Ответ написан
Комментировать
ShadowOfCasper
@ShadowOfCasper
Middle User Interface Web Developer
На самом деле парень выше прав. Один из главных аспектов говнокода - профедурный подход. С такой простынёй сложно с первого взгляда понять какие функции за что именно отвечают - разбей их на модули. Сравни. Найди одинаковые значения / вычисления одинаковых значений -> вынеси в отдельные функции -> унаследуй там где они нужны. На вид всё грамотно. Только избегай копипасты типа:
this.coords = {
		top: e.clientY - this.shiftY - this.parentShift.top,
		left: e.clientX - this.shiftX - this.parentShift.left
	};

this.coords.top = e.clientY - this.shiftY - this.parentShift.top;
	this.coords.left = e.clientX - this.shiftX - this.parentShift.left;

Которые у тебя повторяются.
coords создай вне обработчиков. Вычисления вынеси в отдельную функцию. И вызывай обработчиках когда необходимо.
Ответ написан
Комментировать
Ваш ответ на вопрос

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

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