Pomoc - Szukaj - Użytkownicy - Kalendarz
Pełna wersja: [php]/[MySQL] Wyswietlanie newsów z bazy.
Forum PHP.pl > Inne > Oceny
dawhol
Witam jestem poczatkujacy w sprawach php ucze sie dopiero kilkanascie dni smile.gif ale napisałem juz pare skryptów dzisiaj chciałem dac pod ostrzał skrypt do wyswietlania newsów z bazy.

Struktóra bazy danych jest następująca:
ID | TITLE | SHORTNEWS | LONGNEWS | DATA

A o to kod pliku newsy_wyswietl.php
  1. <?php
  2. $mysql = '../conf/mysql.ini.php';
  3.  
  4. /* Za pomocą funkcji if sprawdzamy czy istnieje plik ze zmiennej $mysql, gdy funk
    cja zwróci wartosc TRUE
  5.    wczytujemy plik w innym przypadku wyswietlamy blad. */
  6.  
  7. if ( file_exists($mysql)) 
  8. {
  9. require_once($mysql);
  10. } else echo ('Nie można się połączyć z bazą ponieważ plik'.$mysql.' nie istnieje.');
  11.  
  12. /* Deklarujemy funkcje shortnews() */
  13. function shortnews()
  14. {
  15. /* ile na strone */
  16. $ile = 10;
  17. $numrows = mysql_num_rows(mysql_query("SELECT * FROM cms_news"));
  18. if(!$p) $p = 0;
  19. /* zabezpieczenie przed nienumerycznymi wartosciami */
  20. $p = (int)$p; 
  21. $ile = (int)$ile;
  22.  
  23. $zapytanie = "SELECT * FROM cms_news ORDER by data LIMIT $p,$ile";
  24. $wykonaj = mysql_query($zapytanie);
  25.  
  26. while($dane=mysql_fetch_array($wykonaj)) { echo "<table width="500px"><tr>
  27. <td>".$dane['title']."</td></tr>
  28.  <tr><td>".$dane['shortnews']."<br><a href="news_wyswietl.php?wiecej=1&id=".$dane['id']."">Więcej ...</a></td></tr>
  29. <tr><td>".$dane['data']."</td>
  30. </tr></table><br /><br />";
  31. }
  32.  
  33. echo "strona: ";
  34. for($i=0;$i<ceil($numrows/$ile);$i++) {
  35. echo '<a href="'.$PHP_SELF.'?p='.($i*$ile).'">'.($i+1).'</a> ';};
  36.  
  37. }
  38.  
  39. /* Deklarujemy funkcję longnews() */
  40. function longnews()
  41. {
  42.  
  43. if (is_numeric($_GET['id'])) 
  44. {
  45. $id = $_GET['id'];
  46. }
  47. else
  48. {
  49. $id = rand(); /* Jeżeli id nie bedzie liczba lub bedzie puste wtedy zostanie wylosowane
  50.  jezeli nie bedzie go w bazie to nic sie nie wyswietli a 
  51.  jezeli dany id bedzie w bazie wyswietli on w formie dludiej newsa o wylosowanym
     id */
     
  52. };
  53.  
  54. $zapytanie2 = "SELECT * FROM cms_news WHERE id=$id LIMIT 1";
  55. $wykonaj2 = mysql_query($zapytanie2);
  56.  
  57. if ($dane2=mysql_fetch_array($wykonaj2)) { echo "<table width="500px"><tr>
  58. <td>".$dane2['title']."</td></tr>
  59.  <tr><td>".$dane2['shortnews']."<br /><br />".$dane2['longnews']."</td></tr>
  60. <tr><td>".$dane2['data']."</td>
  61. </tr></table>";};
  62. }
  63.  
  64. /* Jezeli zmienna $wiecej jest pusta wtedy zostana wyswietlone wszystkie newsy w 
    formie krótkiej,
  65. jezeli wartos ta bedzie liczba oraz bedzie równa 1 wtedy zostanie wyswietlony je
    den news w całości,
  66. w innym wypadku zostanie wyświetlony bład */
  67.  
  68. $wiecej = $_GET['wiecej'];
  69.  
  70. if (empty($wiecej)) 
  71. {
  72. shortnews();
  73. } 
  74. else 
  75. {
  76. if (is_numeric($wiecej) && $wiecej = 1) 
  77. {
  78. longnews();
  79. } 
  80. else echo ('Błąd wartość zmiennej $wiecej jest nie prawidłowa !!!');  
  81. };
  82.  
  83. ?>


Proszę o ocenę i ew. propozycje poprawy i zmian smile.gif
Seth
Jak na jeden z pierwszych skryptow wyglada niezle winksmiley.jpg


Zmienne, ktore maja byc stale, a do tego sa w jakis spsob konfiguracja w skrypcie (jak $ile = 10) powinny byc definiowane przez define() na poczatku skryptu.
Jezeli bedziesz chcial zmienic liczbe wyswietlanych newsow, to nie bedzie szukal po calym pliku tej zmiennej.


Losowanie newsa raczej jest zbedne. Nie podales przedzialu losowania, wiec raczej napewno wylosuje nieistniejacy. Moze lepiej wyswietlic news z ID 1 ?


Inna sprawa to taka, ze mozna by sie pokusic o jakis system wzorcow aby odseparowac, warstwe prezentacji. Bedzie czytelniej i latwiej w utrzymaniu (zmianach).


Ostatnia chyba sprawa to nazewnictwo zmiennych i komentarze.

$p, $ile nic nie mowia. Jezeli wrocisz do tego skryptu po jakims czasie, to tez Ci one nic nie powiedza (bez analizy kodu).

Dodatkowo, mozna by sie pokusic o inna firme zapisu zmiennych.
Np:
- $alaMaKota albo
- $iAlaMaKota lub $intAlaMaKota.

Pierwszy spsob wyroznia czlowny wyrazow zmiennej. Jest to znacznie czytelniejsze niz ciag nazwa pisana cala z malych liter $alamakota.

Drugi przyklad wprowadza na poczatku nazwy zmiennej infromacje o typie zmiennej (o ile mnei pamiec nie myli, nazywa sie to notacja wegierska?).
i dla int
f dla float
s dla stringa
o dla object itd.

Mozna tez uzyc dluzszej formy typu $intCos, $strTekst, $floatZmienna


Komentarze, ktore sa jedna linia nie trzeba otaczac /* */ do tego jest // na poczatku linii.
Niby mala roznica ale ulatwia np. odkomentowywanie danych linii.


Polecam Ci poczytac takze o standardach kodowania.
http://wortal.php.pl/wortal/artykuly/pomys...dardy_kodowania
Dzieki temu wprowadzisz do swoich skryptow, pewien porzadek i przemyslana strukture.

Milej lektury smile.gif
dawhol
Co do zmian to je wprowadze u siebie tu juz nie bede pisal smile.gif a co do standartów kodowania to chyba tak zle nie jest starałem sie pisac tak zeby bylo czytelnie i czesc z tego co wyczytalem w artykule to juz mam w tym skrypcie zrobione winksmiley.jpg

Ale bede staral sie to lepiej formatowac winksmiley.jpg

Dzięki za opinie smile.gif

a jak jescze mozesz to napisz cos a propos zabezpieczen tzn w sumie niema co tu zabezpieczacc ale chodzi mi o to uzycie is_numeric czy Ci sie podoba winksmiley.jpg itd winksmiley.jpg
SHiP
Jak na kilkanaście dni to wymiatasz winksmiley.jpg Mnie kolega do bazy danych przekonywał z 2 miesiące winksmiley.jpg

  1. <?php
  2. $wiecej = $_GET['wiecej'];
  3.  
  4. if (is_numeric($wiecej) && $wiecej = 1) 
  5.  
  6. {
  7.  
  8. longnews();
  9.  
  10. } 
  11.  
  12. else echo ('Błąd wartość zmiennej $wiecej jest nie prawidłowa !!!');
  13. ?>

Tego kodu nie rozumiem ;p przekombinowałeś ;] Ja bym to napisał w 2 linijkach
  1. <?php
  2. if(isset($_GET['wiecej'])) longnews();
  3. else shortnews();
  4. ?>

Co mnie obchodzi czy ktoś w linku napisze index.php?wiecej=1 czy index.php?wiecej=oTakChceWiecej ważne aby zmienna miała wartość true (no zeby w ogole byla zadeklarowana)

notacja węgierska => http://pl.wikipedia.org/wiki/Notacja_węgierska moze nie jest stworzona do php(brakuje w nim kilku bajerow) ale zawsze mozna sie pokusić o stosowanie, Nie wiem jak ty ale ja po pewnym czasie doszedłem do wniosku że polskie nazwy zmiennych są be winksmiley.jpg i teraz pisze po en i kod jest bardziej dostępny dla obcokrajowcow ;]
dawhol
~SHIP a wiec w tym kodzie chodzi o to iż za pomocą funkcji is_numeric sprawdzam czy zmienna $wiecej jest liczba jezeli tak to czy jest jeden bo w innych wypadkach gdy zmienna jest np literami wyswietli nam bład smile.gif chodzi tu o to iz chce w jaki kolwiek sposób zabepiczyc skrypt przed atakami SQL Iniection czy jakos tak sie to pisało smile.gif wiem ze w sumie niema tu co zabezpieczac bo hasla sie za pomoca tego nie wyjmie tzn w tym skrypcie bo tu niema jak narazie hasla winksmiley.jpg ale to tak zebym juz pamietam o tym zabezpieczenia a jescze jezeli wartosc bedzie liczba inna niz 1 to wtedy zostanie wyswietlony tylko pierwszy news w całosci.

Liczba 1 działa jak TRUE a pozostałe jak FALSE

a co bo baz danych to wydaja mi sie o wiele latwiejsze niz pliki smile.gif a pozatym przedewszystkim sa bezpieczniejsze winksmiley.jpg no i wydaje m isie za bardziej funkcjonalne winksmiley.jpg
SHiP
Mylisz pojęcia ;] sql injection jest mozliwe tylko gdy zmienna jest użyta w zapytaniu SQL np...

  1. <?php
  2. mysql_query('SELECT * FROM tabelka WHERE id="'.$_GET['id'].'"');
  3. ?>

Taki kod jest niebezpieczny i w tym przypadku przydałoby sie sprawdzić $_GET['id']

U ciebie zmienna $_GET['wiecej'] jest tylko sprawdzana pod względem wartości, nie jest przekazywana do jakiegokolwiek zapytania wiec moim zdaneim twoje rozwiązanie jest przekombinowane ;] co za różnica czy zmienna $_GET['wiecej'] bedzie cyfrą czy nie i tak skrypt nie wygeneruje żadnych błedów i nie da okazji na sql_injection ;]
A skoro nie widać różnicy to po co przepłacać? To zawsze tych kilka microsekund ;]
dawhol
Spoko masz racje moj blad (brak wiedzy) ale cóz czlowiek uczy sie na błędach smile.gif
zaraz w takim razie to poprawie winksmiley.jpg i bede pamiętał na następny raz. smile.gif

  1. <?php
  2. if (isset($wiecej) && $wiecej = 1) longnews();
  3. else shortnews();
  4. ?>


tak moze byc smile.gif ?

ps. czyli to:

  1. <?php
  2. function longnews()
  3. {
  4.  
  5. if (is_numeric($_GET['id'])) 
  6. {
  7. $id = $_GET['id'];
  8. }
  9. else ...
  10. ?>
mam zostawic bo ta zmienna jest wstawiana do zapytani sql smile.gif
SHiP
tak winksmiley.jpg tylko
  1. <?php
  2. if (isset($wiecej) && $wiecej = 1) longnews();
  3. else shortnews();
  4. ?>

uciąłeś jeden znak '='. Pozatym nie wiem po co montować dodatkową zmienną $wiecej ;] lepiej sie odwołać do $_GET['wiecej']
dawhol
Spoko smile.gif dzięki za podszkolenie smile.gif
To jest wersja lo-fi głównej zawartości. Aby zobaczyć pełną wersję z większą zawartością, obrazkami i formatowaniem proszę kliknij tutaj.
Invision Power Board © 2001-2025 Invision Power Services, Inc.