Witam,
Zaczynam pisać w OOP, pomaga mi w tym kolega doradzając conieco. Chciałbym byście pomogli mi w pisaniu lepszego kodu. Przestawię wam moje wypociny.

[PHP] pobierz, plaintext 
<?php
class pogoda{
 
    public $web;
    public $cities;
    public $temp;
    public $hpa;
    
    public function __construct($city){
 /*
Wpisane wszystkie miasta które są dostępne w polsce
*/
     $this->cities = array('Aleksandrów Kujawski' => 11642, 'Augustów' => 11643, 'Bartoszyce' => 11644, 'Bełchatów' => 11646, 'Będzin' => 11645, 'Biała Podlaska' => 11647, 'Białka Tatrzańska' => 11648, 'Białobrzegi' => 11649, 'Białogard' => 11650, 'Białystok' => 11651, 'Bielice' => 11652, 'Bielsk Podlaski' => 11653, 'Bielsko-Biała' => 11654); 
 
     
     
        if(array_key_exists($city, $this->cities)){
            $this->web = file_get_contents('http://pogoda.interia.pl/miasta?id='.$this->cities[$city]);
            preg_match_all('/<b>([0-9]+)</b>/<span class="tex2B"  style="font-size:14px;">([0-9]+)</span>/<span class="tex3B">([0-9]+)</span>/', $this->web, $this->temp);
            preg_match_all('/<b>([0-9]+)</b> hPa/', $this->web, $this->hpa);
        }else{
            echo 'Brak miasta!';
        }
        
    }
    
    public function show_cities(){
            $text = '<select>';
        foreach($this->cities as $key => $value){
            $text .= '<option value="'.$value.'">'.$key.'</option>';
        }
            $text .= '</select>';
            
        return $text;
    }
    
    public function today_temp($mi = 0){
              if($mi = 0){
                  $text = $this->temp[1][0];
              }else{
                $text = $this->temp[3][0];
              }
         return $text;
    }
    
    public function temp($mi = 0, $li = 0, $day = 0){
              
            $on = ($mi == 0) ? 1 : 3;
            
            switch($day){
                case 0: if($li == 0) $of = 2; elseif($li == 1) $of = 3; else($li == 2) $of = 4; // przed południem, po południu, wieczorem
                          break;
                case 1: if($li == 0) $of = 5; elseif($li == 1) $of = 6; elseif($li == 2) $of = 7; else $of = 8; // nad ranem, przed południem, po południu, wieczorem
                          break;
                case 2: if($li == 0) $of = 9; elseif($li == 1) $of = 10; elseif($li == 2) $of = 11;  else $of = 12; // nad ranem, przed południem, po południu, wieczorem
                          break;
                case 3: $of = ($li == 0) ? 13 : 14;  // dzien, noc
                          break;
                case 4: $of = ($li == 0) ? 15 : 16;   // dzien, noc
                          break;
                case 5: $of = ($li == 0) ? 17 : 18;   // dzien, noc
                          break;
            }
            
         return $this->temp[$on][$of];
    }
    
    public function hPa($mi = 0){
    
        return $this->hpa[1][$mi];
    
    }
    
    
    public function __destruct() {
        unset($this);
    }
    
    
    
    
}
 
 
    $obiekt = new pogoda('Warszawa');
    $obiekt->show_cities();
    echo $obiekt->temp(1,1,1);
    echo $obiekt->hPa(2);
?>
[PHP] pobierz, plaintext 

[PHP] pobierz, plaintext 
<?php
$obiekt->show_cities();
?>
[PHP] pobierz, plaintext 

raczej

[PHP] pobierz, plaintext 
<?php
echo  $obiekt->show_cities();
?>
[PHP] pobierz, plaintext 

Ale i tak widzę poprawiłeś dużo, o czym mówiłem. ;d

Dziwna jest ta funkcja temp()

[PHP] pobierz, plaintext 
<?php
case 0: if($li == 0) $of = 2; elseif($li == 1) $of = 3; else($li == 2) $of = 4; // przed południem, po południu, wieczorem
                         break;
               case 1: if($li == 0) $of = 5; elseif($li == 1) $of = 6; elseif($li == 2) $of = 7; else $of = 8; // nad ranem, przed południem, po południu, wieczorem
                         break;
               case 2: if($li == 0) $of = 9; elseif($li == 1) $of = 10; elseif($li == 2) $of = 11;  else $of = 12; // nad ranem, przed południem, po południu, wieczorem
?>
[PHP] pobierz, plaintext 

w każdym case $of jest proporcjonalna do $li, zamień to na

[PHP] pobierz, plaintext 
<?php
case 0: $of = $li+2;                         break;
               case 1: $of = $li+5                         break;
               case 2: $of = $li+9
?>
[PHP] pobierz, plaintext 

i będzie krócej tak jak chciałeś ;]

Krócej to byłoby zdefiniować stałe w klasie np.

rano, poludnie, wieczor

nadać im wartości i używać w switchu po prostu stałych i tak samo przy wywołaniu metody.

[PHP] pobierz, plaintext 
<?php
public function __construct($city){
/*
Wpisane wszystkie miasta które są dostępne w polsce
*/
    $this->cities = array('Aleksandrów Kujawski' => 11642, 'Augustów' => 11643, 'Bartoszyce' => 11644, 'Bełchatów' => 11646, 'Będzin' => 11645, 'Biała Podlaska' => 11647, 'Białka Tatrzańska' => 11648, 'Białobrzegi' => 11649, 'Białogard' => 11650, 'Białystok' => 11651, 'Bielice' => 11652, 'Bielsk Podlaski' => 11653, 'Bielsko-Biała' => 11654);
?>
[PHP] pobierz, plaintext 

Przecież te zmienne możesz zadeklarować bezpośrednio przy definicji zmiennej, po co w konstruktorze?

[PHP] pobierz, plaintext 
<?php
public function show_cities(){
           $text = '<select>';
       foreach($this->cities as $key => $value){
           $text .= '<option value="'.$value.'">'.$key.'</option>';
       }
           $text .= '</select>';
           
       return $text;
   }
?>
[PHP] pobierz, plaintext 

A czemu mieszasz logikę z wyglądem? MVC!

[PHP] pobierz, plaintext 
<?php
public function temp($mi = 0, $li = 0, $day = 0){
             
           $on = ($mi == 0) ? 1 : 3;
?>
[PHP] pobierz, plaintext 

No ok, ale co mają powiedzieć te zmienne? MI/LI? Coś z listą wyliczeniową? Będziesz w stanie zrozumieć to za miesiąc, gdy "odpoczniesz" od kodu? PS. Zainteresuj się składnią phpDoc.

[PHP] pobierz, plaintext 
<?php
public function __destruct() {
       unset($this);
   }
?>
[PHP] pobierz, plaintext 

A to już totalny bezsens.

Można tak, można tak - kwestia przyzwyczajenia, jednak wersja z konstruktorem wydaje się być lepsza. ;d


Wcześniej były echa, ja mu doradziłem aby zmienił na return.


Zgadzam się.


Zwolnienie pamięci uważasz za bezsens? Gdyby to był większy system, też byłby to bezsens?
To tak jak np. z zamykaniem połączenia mysql, jak nie użyjesz mysql_close() skrypt i tak będzie działał.

Śmiem się sprzeczać. Niby pod jakim względem lepsza? Potrafisz uzasadnić? Masz jakieś argumenty?


To nie jest na to miejsce. Widok/helper - owszem. Ale nie logika.


Eee, to jest inna sprawa. Zwalnianie zasobów SQL, to co innego. Poza tym, skrypt przy końcu robi to sam.

Poczytaj, kiedy jest wyzwalany destruktor, to wtedy porozmawiamy, czy jest sens korzystania z unset" title="Zobacz w manualu PHP" target="_manual na $this. [;

Jeżeli najdzie mnie myśl pobierania z konstruktora jakiegoś parametru to tylko w konstruktorze podmienię, nie muszę grzebać w reszcie kodu. ;p
Z resztą zazwyczaj ludzie tak piszą, że w klasie tylko definiują zmienne a w konstruktorze nadają im domyślne wartości.



Podczas niszczenia obiektu. A czy to taki duży błąd...? Nie, ale fakt mogłoby go nie być.

Zależy. Naprawdę zależy. Ale skoro to jest statyczna tablica, to dlaczego przy definiowaniu typów zmiennych nie wrzucasz wszystkich do jednego worka (np. dajesz null, null, null) i im typ przypisujesz dopiero w konstruktorze? A co by było, gdybyś chciał przerobić klasę na statyczną? Wtedy masz więcej roboty.


BUZI -> http://pl.wikipedia.org/BUZI

Po prostu wszystko zależy od przeznaczenia klasy.


Hahaha, spodobało mi się. Poprawiłeś mi humor.