[MySQL][php] Rotator Banerów =) Oceńcie i sprawdzcie :) Opcje

[dawhol]

W tabeli mamy trzy pola:
"adres" - adres do strony
"obrazek" - adres do obrazka
"wyswietlen" - ilos wyswietlen

Pliku z formularzem nie daje bo chyba kazdy wie jak wygląda
Pliku gdzie jest wykonywana funkcja która dodaje do bazy MySQL tez nie daje poniewaz tego nie chce abyscie oceniali bo niema tam co zwykle zapytani zalezy mi na ocenie skryptu który wyświetla banery rotacyjnie co odswierzenie strony . Oto jego kod:

[PHP] pobierz, plaintext 
<?php
$zapytanie = "SELECT * FROM cms_bannery WHERE id BETWEEN 1 AND 3 AND wyswietlen<1000 ORDER BY rand() LIMIT 1";
$wykonaj = mysql_query($zapytanie);
 
if($dane=mysql_fetch_array($wykonaj)) 
{
	echo ('<a href="'.$dane['adres'].'"><img src="'.$dane['obrazek'].'"></a>');
} 
else 
	 echo mysql_error();
 
$wysw = $dane['wyswietlen'];
$adres = $dane['adres'];
$up = 1;
 
$ilwysw = $wysw+$up;
 
$zapytanie2 = "UPDATE cms_bannery SET wyswietlen='$ilwysw' WHERE adres='$adres'";
$wykonaj2 = mysql_query($zapytanie2);
 
if($wykonaj2) 
{
	echo ('');
} 
else 
	echo mysql_error();
?>
[PHP] pobierz, plaintext 

Oczywiscie skrypt po kazdym wyswietleniu banera dodaje 1 do jego liczby wyswietlen

Prosze o oceny i ew. rady co i jak poprawic moj drugi skrypcik po 4-5 dniach nauki

Ten post edytował dawhol 25.07.2006, 08:14:14

[nospor]

[PHP] pobierz, plaintext 
<?php
if($dane=mysql_fetch_array($wykonaj)) 
{
	echo ('<a href="'.$dane['adres'].'"><img src="'.$dane['obrazek'].'"></a>');
} 
else 
	 echo mysql_error();
?>
[PHP] pobierz, plaintext 

Te mysql_error() powinno byc po mysql_query a nie po mysql_fetch_array(). W przypadku bledu zapytania dostaniesz jeszcze niepotrzebne bledy przy mysql_fetch_array() gdyz wykonywac sie bedzie na blednych danych.
Gdy zapytanie nic ci nie zwroci, to wygenereuje sie mysql_error(), czyli nic sie nie wygeneruje, gdy errora mysql nie bedzie.
Krotko mowiac: w zlym miejscu dalej mysql_error()

[SQL] pobierz, plaintext 
...id BETWEEN 1 AND 3...
[SQL] pobierz, plaintext 

czemu tak? mozna tylko 3 banery losowac i to tylko z zadanym id?

[dawhol]

stwierdzilem terasz ze trzeba by poprawic to tak aby wszystko wzgledem ID bylo albo obrazka a nie adresu bo czasem mozna dac dwa rozne banery pod jeden adres =)

w sumie trzeba by to popraiwc tam jes BETWEN 1 and 3 poniewaz chce aby losowal mi liczbe pomiedzy tymi trzema bo obecnie mam 3 banery w bazie ale trzeba by tam dac ostatni id z bazy zamiast tej 3 zapomniałem to zrobic

[nospor]

ale trzeba by tam dac ostatni id z bazy

Ale po co? Po co wogole warunek na te id, skoro mają byc losowane wszystkie rekordy? wystarczy warunek na wyswietlenia.

[dawhol]

no spoko ale jak nie bylo tego to mi losowal liczbe której niema w bazie a moze mialem inny blad
musze to sprawdzic poziej bo teraz lece do pracy

[nospor]

no spoko ale jak nie bylo tego to mi losowal liczbe której niema w bazie

jak moze losowac ci z bazy cos czego nie ma? Losuje po rekordach, ktore są

[Astarot]

W swoim rotatorze nie uwzględniłeś danych osadzonych w skryptach JS, a także banerków flashowych. Pozatym dodaj alt="coś" bo walidator będzie krzyczał.

<img src="'.$dane['obrazek'].'" alt="cos" />

pozdr.

Ten post edytował Astarot 25.07.2006, 09:27:42

[dawhol]

~nospor moze i masz racje widocznie mialem inny blad i jak go poprawilem to zapomnialem tego zmienic no ale w koncu ucze sie od paru dni wiec odrazy nie napisze skryptół bezbłędnyc po to je tu daje zeby je ocenic i ew. podac jakies uwagi

~astarot fatk zapomniałem o tym zdarza sie trzn w pierwszej wersji to było ale w pierwszej wersji nie mialem za to ilosci wyswietlen a teraz dodalem i zapomnialem o tym

[Nightwalker]

w Twoim kodzie nie ma jakże ważnego

mysql_free_result" title="Zobacz w manualu php" target="_manual

[dawhol]

~Nightwalker zaraz to dodam u siebie dzieki za poinformowanie mnie nawet nie znalem takiej funkcji ale teraz juz bede o niej pamietał w koncu czlowiek uczy sie na blędach

[nospor]

bez przesady... jak juz podajecie linki to czytajcie chociaz:

mysql_free_result() używa się tylko w wypadkach obawy zajęcia zbyt dużej ilości pamięci przez zapytania zwracające duże ilości danych. Cała pamięć przydzielona wynikowi skojarzonemu z podanym identyfikatorem wyniku będzie automatycznie zwolniona.

Skrypt tutaj zwraca jeden rekord, ktory ma sie nijak do " duże ilości danych."

edit (po poscie ponizej): z tresci posta wnioskowalem jednak ze to jest wymog dla tego skryptu a nie dobry nawyk

[Nightwalker]

bez przesady... jak juz podajecie linki to czytajcie chociaz:

Skrypt tutaj zwraca jeden rekord, ktory ma sie nijak do " duże ilości danych."

Takie coś rodzi złe nawyki, dlatego to napisałem

Edit do posta powyżej: źle się wyraziłem, ale lepiej opróżniać ZAWSZE i później nie zapominać, niż na odwrót (przynajmniej takie jest moje zdanie w tej sprawie)

Ten post edytował Nightwalker 25.07.2006, 17:33:59

[Turgon]

Według mnie przydałoby się mysql_num_rows, gdyż wykonujemy selecta id z bazy bez warunków, czyli mamy wszystkie rekordy kolumny id. Następnie ta prosta komenda i skrypt wie spośród jakich liczb losowac.

[dawhol]

Dobra to powiem co poprawilem i moderator moze zamknac temat jak chce ofc

1) usubołem warunek id BETWEEN 1 AND ... i działą moj bład był jakis
2) dodałem pole opis w bazie oraz alt w obrazku i title w linku
3) zrobiłem echo mysql_error w funkcji warunkowej przy mysql_query tak jak radzil ~nospor
4) dodałem w tabeli pole max wyswietlen i zamiast 1000 w pierwsyzsm zapytaniu sql dalem 'maxwyswietlen' tzn:

[SQL] pobierz, plaintext 
SELECT * FROM cms_bannery WHERE wyswietlen<maxwyswietlen ORDER BY rand() LIMIT 1
[SQL] pobierz, plaintext 

działa pięknie jeszcze pozniej sie pobawei aby wyszczególnic banery gif od flasha i innych ale to pozniej

Ten post edytował dawhol 25.07.2006, 17:19:51

[Nightwalker]

I jeszcze jedno, dlaczego wyciągasz wszystko z bazy (SELECT *) zamiast wyciągnąć tylko potrzebne pola, przykładowo

[SQL] pobierz, plaintext 
SELECT `pole1`, `pole2`, `pole3` FROM `tabela`
[SQL] pobierz, plaintext 

?

Komentarz do wpisu poniżej: nie znam struktury bazy, a przeważnie potrzebne jest tylko kilka pól...

Ten post edytował Nightwalker 25.07.2006, 18:52:49

[dawhol]

no bo wszystko jest mi potrzbne to chyba logiczne