Codereview początków z Js'em!

Tak na szybko dwie rzeczy bym zmienił:

1. nie dawać na sztywno wpisanych wartości (np. text.style.fontFamily = 'arial';) ale gdzieś zdefiniować określone style/zmienne, a potem się do nich odwoływać. Obecnie jest to proste i ewentualne zmiany nie będą problemem, ale w wypadku bardziej rozbudowanych skryptów, jakbyś chciał zmienić Arial na Verdana to musiałbyś poprawiać w dziesiątkach/setkach miejsc. Mając to w jakiejś stałej/zmiennej poprawiasz raz i masz po temacie

2. fragment

btnBlack.addEventListener('click' , function(){
  console.log('Zmieniono na czarny');
  text.style.color = 'black';
})

btnPink.addEventListener('click' , function(){
  console.log('Zmieniono na różowy');
  text.style.color = 'pink';
})

Masz tutaj w dwóch miejscach powtórzenie tego samego, tylko z innymi wartościami.
Analogicznie do pkt. 1: jeśli później będziesz chciał wprowadzić jakieś zmiany, to mając to tak rozbite, będziesz musiał poprawiać w każdym miejscu osobno. Bardziej sensowne wydaje się tutaj stworzenie jednej funkcji, którą wywołasz z odpowiednim parametrem - dla pierwszego przypadku byłoby to coś w stylu zmienTlo (black) a drugim zmienTlo (pink). W ten sposób możesz też w bardzo łatwy sposób dodać kolejne kolory.

Poczytaj sobie wątek https://4programmers.net/Forum/Off-Topic/Oceny_i_recenzje/312543-prosta_gra_w_js_code_review a w szczególności post https://4programmers.net/Forum/1501353

Możesz np. do przycisków dodać atrybut data z kolorem np. <button data-color="red">Czerwony</button> i na tej podstawie wystarczy, że podepniesz się pod klasę a nie nasłuchujesz każdy przycisk osobno.

tak z nudow zrobilem cos podobnego, jak chcesz mozesz sobie rzucic okiem https://codepen.io/anon/pen/KxgJaQ

anonimowy napisał(a):

Możesz np. do przycisków dodać atrybut data z kolorem np. <button data-color="red">Czerwony</button> i na tej podstawie wystarczy, że podepniesz się pod klasę a nie nasłuchujesz każdy przycisk osobno.

Wiem, że zwykłe onclick jest mniej elastyczne niż podpinanie akcji z zewnątrz, niemniej w takim prostym przypadku i skoro celem ma być zwięzłość, to czemu nie zrobić po prostu:

<button onclick="zmien(0, 'red', 0);">Czerwony</button>

function zmien {tlo, kolor, czcionka) {
...
}

?

1. Oddzielenie zachowania od znaczników

Celem znaczników HTML jest opisanie struktury dokumentu, a nie jego zachowania.

Wymusza powiązanie między niektórymi znacznikami i niektórym kodem JS, przez co nie pozwala na zamianę JavaScriptu bez zmiany znaczników. To jest ból głowy podczas utrzymania i zwiększa ryzyko błędów w produkcji.

1. Jest trudne do utrzymania

Bariera do rozszerzania. Bardzo powszechną (codzienną właściwie) rzeczą jest dostarczanie dodatkowych funkcji do istniejącej aplikacji, ale ta konwencja może sprawić, że aplikacja będzie bardziej skomplikowana i kosztowniejsza.

Przechowywanie kodu HTML oddzielnie od kodu CSS, oddzielenie od kodu JavaScript jest bardziej przejrzyste, łatwiejsze i jest powszechnie uznawane za najlepszą praktykę.

anonimowy napisał(a):

1. Oddzielenie zachowania od znaczników
2. Jest trudne do utrzymania

Dziękuję za odpowiedź. Faktycznie to ma sens w przypadku kodu, który będzie / ma szansę być elementem jakiegoś większego projektu, ale czasem robi się też jakieś maleństwa do jednego prostego celu.

Tylko, że moim zdaniem ta porządniejsza wersja jest mimo wszystko czytelniejsza. I zawsze dobrze jest się trzymać dobrych praktyk.