Animowana galeria z filtrowaniem - jQuery plugin

0

Witam,

chciałbym poddać Waszej ocenie mój open source'owy projekt, który jakiś czas temu rozpocząłem i powolutku sobie rozwijam.
Będę wdzięczny za informacje, wszelkie uwagi na temat jakości kodu, zmian i czegokolwiek innego co Wam przyjdzie do głowy.
Niedawno zacząłem przygodę z JS, więc domyślam się, że uwag może być sporo :)
Za wszystkie bardzo dziękuję.

Przesylam link: http://agileplayers.github.io/agileGallery/

Pozdr.
Kamil

1

Co się o d razu rzuca w oczy to:

/***** global variables *****/
var initialPositions = 'blank';
var screenWidth = 'blank';
var previousClassFilter = 'blank';

oraz:

/***** private methods *****/
function getElementCurrentPosition(element) {
[...]

To wszystko sa globale (szczególnie ciekawe jest nazwanie globalnych funkcji prywatnymi metodami O_o ). Pozbądź się tego.
Także większość metod z konstruktora (wszystkie?) powinieneś wyrzucić do prototypu - inaczej tworzysz ich kopie z każdym utworzeniem obiektu.
Ogólnie to masz problem z widocznością zmiennych.

Kilka smaczków:

if (screenSize > 1200) {
    numberOfElementsPerRow = elementsPerRowAllSizes.screenLg;
} else if (screenSize < 1200 && screenSize >= 992) {
    numberOfElementsPerRow = elementsPerRowAllSizes.screenMd;
} else if (screenSize < 992 && screenSize >= 768) {
    numberOfElementsPerRow = elementsPerRowAllSizes.screenSm;
} else if (screenSize < 768) {
    numberOfElementsPerRow = elementsPerRowAllSizes.screenXs;
};

Po co Ci dwa warunki w else if?? Nie lepiej:

if (screenSize > 1200) {
    numberOfElementsPerRow = elementsPerRowAllSizes.screenLg;
} else if (screenSize >= 992) {
    numberOfElementsPerRow = elementsPerRowAllSizes.screenMd;
} else if (screenSize >= 768) {
    numberOfElementsPerRow = elementsPerRowAllSizes.screenSm;
} else {
    numberOfElementsPerRow = elementsPerRowAllSizes.screenXs;
};

Pomijam już to, że pewnie da się tych ifów całkiem pozbyć.

typeof screenXs === 'undefined' || isNaN(screenXs) => !screenXs

$('.' + className).css('position', 'absolute');
$('.' + className).css('position', 'initial');
$('.' + className).css('display', displayType);
$('.' + className).css('position', 'absolute');
$('.' + className).css('position', 'initial');

Naprawdę pobierasz ten sam element za każdym razem i pojedynczo nadajesz mu style? Masz tak wszędzie w kodzie. Toż to zbrodnia (wydajność).

i = i + 1;

:D Nieźle podsumowuje to rozwlekłość kodu;)

Na plus ładne nazwy zmiennych i czytelność kodu.

0

Witam,

zgodnie z otrzymanymi uwagami, poprawiłem kod.
Wprowadzone zmiany to:

  • usunięcie zmiennych i metod globalnych;
  • przeniesienie metod do prototypu;
  • ujednolicenie nazw parametrów w metodach;
  • drobna refaktoryzacja;

Będę wdzięczny za weryfikację, ocenę i wszelkie dalsze uwagi.
Dzięki raz jeszcze za pomoc :)

Pozdr.
Kamil

1 użytkowników online, w tym zalogowanych: 0, gości: 1