JS - ocena biblioteki do updatowania DOM

Question

Cześć, na początku czerwca prosiłem o ocenę, jednak od tego czasu sporo rzeczy poprawiłem i wykrystalizował mi się bardziej spójny i sensowny obraz tego mini projektu oraz stworzyłem przykład prezentujący możliwości libki, stąd prośba o ponowną ocenę.

Cel biblioteki to ułatwienie updatowania elementów DOM, na podstawie wcześniej ustalonych danych i logiki.

Przykładowy problem to implementacja slajdera z dwiema kategoriami tematycznymi: aktorzy i filozofowie, po 5 zdjęć na kategorię,
oraz umożliwienie użytkownikowi interakcji z nim i odpowiednie reagowanie DOM na jego poczynania.

Chcemy:

• co 5 sekund losować slajdy unikając powtarzalności do 3 elementów w tył
• podczas zmieniania slajdu uzyskać efekt fadeIn
• umożliwić zatrzymanie slidera po kliknięciu na odpowiedni paragraf oraz restart po drugim kliknięciu
• utworzyć powiązanie między kategorią tematyczną slajdera a napisem na przycisku do zmiany tematu slajdów 
• utworzyć powiązanie między tym czy slajder działa czy jest zatrzymany a napisem pod slajderem
• W przypadku paragrafu i buttona do zmiany tematu chcemy tylko doklejać dane do ich HTML a nie całkowicie go zmieniać

Github z kodem i linkiem do przykładu

Co sądzicie o kodzie źródłowym oraz o samej bibliotece?

Dzięki za odpowiedzi

Answer 1

Parę  uwag na szybko, bo nie mam zbytnio czasu na dokładne analizowanie kodu:

1. zbyt duży plik, podziel to na mniejsze pliki i przemyśl jakąś ich sensowną strukturę.
2. zbyt mocno łaczysz html z js... JS powinien interesować się tym co mają robić dane elementy, a nie tym jakie to są znaczniki i jak wyglądają. Nawet jeśli całość miałaby być generowana w JS to wg mnie powinno to być rozdzielone na elementy odpowiedzialne za stworzenie struktury, ostylowanie i trzeci element odpowiedzialny za interakcję.
3. dlaczego raz stosujesz template strings a raz przeciążanie operatora "+"?
4. metody h1, h2... robią praktycznie to samo - po co tyle klas? Tutaj (pomijają w ogóle zasadność tych klas) powinna być jedna metoda tworząca odpowiedni element
5. Skoro używasz składni ES6 to zapis : { cls: cls, content: content } możesz w tym wypadku uprościć do { cls,  content }
6. Unikaj tworzenia metod o nazwach get() i set()... w peirwszej chwili sugerowało mi to,że stosujesz gettery i settery co okazało się nie prawdą :) ale odruchowo tego właśnie się spodziewam widząc get/set
7. Generalnie zbyt dużo tych metod i tego wszystkiego... nie zapanowałbyś nad takim kodem produkcyjnym... postaraj się zrefaktoryzować ten kod, unikaj zbyt mocnego łączenia html, js, css, niech np. klasa odpowiedzialna za obsługę guziczków next i prev (czyli ogólnie interakcję slidera) nie wie jakie znaczniki przedstawuiaja te elementy w html i jak wyglądają.. ma ona tylko odpowiednio obsłużyć kliknięcie w next, bez względu czy będzie to button, div itp.

Popracuj nad czytelnością, modularnością i dobrymi praktykami pisania kodu i daj wersję poprawioną to będziemy szlifować dalej :)

Powodzenia!

Comments

Dziękuję za wskazówki