Co jest źle w tym zegarku?

pytanie zadane 25 września 2016 w JavaScript przez VeniK Początkujący (270 p.)

<!DOCTYPE html>
<html>
<head>
	<title>Strona o niczym</title>
	<meta charset="utf-8" />
	<meta name="author" content="" />
	<meta name="keywords" content="" />
	<meta name="description" content="Testowa strona o czymkolwiek" />
	<link rel="stylesheet" href="style.css" />
	
	<script type="text/javascript">
	function odliczanie(){
	
		var data = new Date();
		var godzina = data.getHours();
		if (godzina<10) "0"+godzina;
		var minuta = data.getMinutes();
		if (minuta<10) "0"+minuta;
		var sekunda = data.getSeconds();
		if (sekunda<10) "0"+sekunda;
	
		document.getElementByClassName("zegar").innerHTML = godzina+":"+minuta+":"sekunda;
		
		setTimeout("zegar()",1000);
	
	}
	</script>
</head>
<body onload="odliczanie();">
<header class="logo">
	<h1><span>Fajne logo</span></h1>
</header>
<header class="zegar"></header>
</body>
</html>

Mam taki kod, nie umiem jeszcze javascriptu, przed chwilą na szybkości oglądnąłem odcinek 1 js Pana Zelenta. Strzelam, że błąd będzie przez header albo getElementByClassName. Tak jak mówiłem, nie znam się na js, ale na pierwszy rzut oka ten tag wydaje się legitny :D

3 odpowiedzi

odpowiedź 25 września 2016 przez xmentor Nałogowiec (49,520 p.)

odpowiedź 25 września 2016 przez niezalogowany

(Literówka) getElementsByClassName - zwraca HTMLCollection pod postacią pseudo-tablicy. Musisz odnieść się do jej pierwszego elementu.
```
document.getElementsByClassName("zegar")[0].innerHTML...
```
Syntax error
```
...innerHTML = godzina+":"+minuta+":"sekunda;
```
brak znaku konkatenacji ("+") przed zmienna sekunda
setTimeout - nie wywołuj eval: (btw, nie masz zadeklarowanej funkcji zegar)
```
setTimeout("zegar()",1000); // DONT
setTimeout( odliczanie , 1000 ) 
```
(wracając do poprzedniego tematu) Wybrałeś jedną z gorszych opcji dla h1
```
<h1><span>Fajne logo</span></h1>
```
Im mniej węzłów w DOM tworzysz, tym lepiej dla Ciebie i przeglądarki. (czytelniejszy kod, szybsze parsowanie, szybszy dostęp z DOM API do elementów)
```
<h1>Fajne logo</h1> 
```
zdecydowanie jest lepszym rozwiązaniem.

komentarz 25 września 2016 przez VeniK Początkujący (270 p.)
edycja 25 września 2016 przez VeniK

Po dodaniu [0] zaczęło działać ;)

Dodaje jeszcze kod jaki teraz mam:

<!DOCTYPE html>
<html>
<head>
	<title>Strona o niczym</title>
	<meta charset="utf-8" />
	<meta name="author" content="" />
	<meta name="keywords" content="" />
	<meta name="description" content="Testowa strona o czymkolwiek" />
	<link rel="stylesheet" href="style.css" />
	
	<script type="text/javascript">
	function odliczanie()
	{
	
		var data = new Date();
		var godzina = data.getHours();
		if (godzina<10) godzina = "0"+godzina;
		var minuta = data.getMinutes();
		if (minuta<10) minuta = "0"+minuta;
		var sekunda = data.getSeconds();
		if (sekunda<10) sekunda = "0"+sekunda;
	
		document.getElementsByClassName("zegar")[0].innerHTML = godzina+":"+minuta+":"+sekunda;
		
	setTimeout( odliczanie , 1000) ;
	
	}
	</script>
</head>
<body onload="odliczanie();">
<header class="logo">
	<h1>Fajne logo</h1>
</header>
<header class="zegar"></header>
</body>
</html>

@import 'https://fonts.googleapis.com/css?family=Julius+Sans+One';
body {
	padding: 0;
	margin: 0;
	background-color: #689F38;
}
header {
	font-size: 24px;
	font-family: 'Julius Sans One', sans-serif;
	color: white;
	margin: 50px;
}
header.logo {
	float: left;
}
header.logo h1 {
	background-color: rgba(0, 0, 0, 0.9);
	border-radius: 10px;
	padding: 5px 10px 5px 10px;
	display: inline-block;
}
header.zegar {
	float: left;
}

Czy wszystko jest w miarę w porządku?

A i jeszcze jedno. Da radę jakoś dodać tag <h1> do tego kodu javascript, żeby zegar wyświetlał się w tym tagu? >>> Zrobiłem to dodając w htmlu do h1 klasę zegar, ale chodzi mi o to, żeby te tagi dodać w kodzie js.

komentarz 25 września 2016 przez niezalogowany

"W miarę w porządku" jak na standardy MZ z kursu JS - tak. Jak na standardy JS - mało rzeczy tu jest w porządku - https://github.com/CodersCommunity/CodeReview-JS-01 (na branchach refactor/ES6 masz poprawny kod)

Jasne, że się da : ) kombinuj - oto chodzi z nauką.

komentarz 25 września 2016 przez VeniK Początkujący (270 p.)

Czytam po internetach, że trzeba użyc document.write aby coś takiego osiągnąć... czy nie ma prostszej wersji? Bo nie wiem jak ten mój kod przerobić w takim razie ;/

komentarz 25 września 2016 przez niezalogowany

zapraszam na IRCa naszego

komentarz 25 września 2016 przez VeniK Początkujący (270 p.)

Nie ma innej opcji? :d Tutaj w komentarzu?

komentarz 25 września 2016 przez niezalogowany

Jest. Tylko wytłumacz dokładnie o co Ci chodzi : )

odpowiedź 25 września 2016 przez Comandeer Guru (602,330 p.)

F12 w przeglądarce i zobacz, co rzuca konsola → i już, problem rozwiązany ;)

Pokaż 8 poprzednich komentarzy

komentarz 25 września 2016 przez Comandeer Guru (602,330 p.)

Wnosi - operujesz na żywym DOM.

Szybki trolling:

var trolls = document.getElementsByClassName( 'troll' ),
    i;

for ( i = 0; i < trolls.length; i++ ) {
    document.body.appendChild( trolls[ i ].cloneNode( true ) );
}

↑ Powyższy kod scrashuje przeglądarkę/taba ;) Dlatego IMO lepiej unikać żywych kolekcji DOM-owych – zwłaszcza jeśli robimy na nich jakieś operacje "mutacyjne".

'98% programistów nie ma pojęcia, czy pracuje na snapshocie, czy nie'

"Programistów"? ;)

komentarz 25 września 2016 przez niezalogowany

for ( i = 0; i < trolls.length; i++ ) {
    ...
}

Troll, nie troll - nie tworzy się takich konstrukcji^ Sprawdzanie długości tablicy przy każdej iteracji, kiedy trzeba lecieć do DOMa, jest niewydajne.

for ( i = 0, max = trolls.length; i < max ...

I teraz:

Po co to komu.. jeśli mamy querySelectorAll
Po co to komu.. jeśli mamy for-of loop

...

Nie lubię z Tobą rozmawiać : )

komentarz 25 września 2016 przez niezalogowany

Swoją drogą, w JavaScript Patterns - Stoyana Stefanova, jest temat, poświęcony właśnie HTMLCollection, i słowa nie ma o querySelector - a szkoda. Podobno polecana lektura.

Edit: Jest, jednak nic nie wnosi.

komentarz 25 września 2016 przez Comandeer Guru (602,330 p.)

Sprawdzanie długości tablicy przy każdej iteracji, kiedy trzeba lecieć do DOMa, jest niewydajne.

Jakiś fetysz z tą wydajnością? :D Uwierz mi, że jak Twoja aplikacja będzie się ciąć, to tego typu rzeczy będą na szarym końcu listy optymalizacji, które należy wprowadzić. Zwłaszcza, że jeśli elementów nie jest niebotycznie dużo, to narzut wydajnościowy jest praktycznie niezauważalny. BTW przypomniałeś mi o http://mrale.ph/blog/2014/12/24/array-length-caching.html ;)

No ale skoro już podałeś taki przykład, to pobawmy się nim ;) https://jsfiddle.net/Comandeer/3prwkrcb/

Chociaż cache length wskazuje na fakt, że chcesz użyć czegoś na kształt snapshota, to "żywość" HTMLCollection i tak Ci nie pozwoli. W przeciwieństwie do prawdziwego snapshota na podstawie NodeList → https://jsfiddle.net/Comandeer/3prwkrcb/1/

Wniosek jest prosty: jeśli chce się mutować DOM (czyli… prawie zawsze), żywe kolekcje są zbyt niestabilne.

Co do for...of loop: niektórzy jej nie polecają, bo to zbyt low level. Ot, niewystarczająca liczba abstrakcji ;)

Co do książki: fakt, jest dobra… ale czy przypadkiem nie powstała przed querySelectorAll?

1	komentarz 25 września 2016 przez niezalogowany Pozamiatane, dzięki za wyjaśnienie - dobrze, że jesteś. (Co do książki - została wydana w 2010, querySelector oszacowałem na 2008 (na podstawie dostępności w poszczególnych wersjach przeglądarek, a datą ich wydania))