obsługa artykułów - pierwsza klasa Opcje

[barii]

Ostatnio zacząłem czytam o programowaniu obiektowym trochę w necie. Napisałem sobie pierwszą klasę, która ma być odpowiedzialna za obsługę artykułów (tworzenie newsów, kategorii, administrację).

[PHP] pobierz, plaintext 
<?
class Articles{
var $id; //nr artukułu
 
 
function remove_news($id){
      $sql = "DELETE FROM rada_news WHERE id = $id";
      $result = mysql_query($sql);
      echo '<p id="succes">Usunięto aktualość o id='.$id.'</p>';
}
 
function add_news($tytul,$img,$kategoria,$tekst){
      $data=Date("Y-m-d");
      $sql = "insert into rada_news  ( tytul, img, kategoria, tekst, data, akcept ) values ( '$tytul','$img', '$kategoria', '$tekst', '$data', '0');";
      $result = mysql_query($sql);
      echo '<p id="succes"><b>Dodano aktualość '.$tytul.'</b></p>';
}
 
function change_st($id,$stan){
      $sql = "UPDATE rada_news SET akcept='$stan' WHERE id=$id";      
      $result = mysql_query($sql);
      if($stan==1) $statut='aktywnty';
      if($stan==0) $statut='niaktywnty';
      echo '<p id="succes">Zmieniono status wiadomości o id='.$id.' na <b>'.$statut.'</b></p>';
}
 
function edit($id,$tytul,$img,$kategoria,$tekst){
      $sql = "UPDATE rada_news SET tytul='$tytul', img='$img', tekst='$tekst', kategoria='$kategoria'  WHERE id=$id";       
      $result = mysql_query($sql);
      echo '<p id="succes">Zapisano poprawnie aktualość: <b>'.$tytul.'</b></p>';
}
 
function pobierz_dane($opcja,$id){
    $sql = "SELECT $opcja as pole FROM `rada_news` WHERE id='$id'";
    //echo $sql.'<br>';
    $res = mysql_query($sql);
    $row = mysql_fetch_array($res);
    return $row['pole'];
}
 
function show_news($id){
      $sql = "SELECT * FROM rada_news WHERE akcept=1 AND id=$id";
      $result = mysql_query($sql);
      $ile = mysql_num_rows($result);
      $row = mysql_fetch_array($result);
      //echo $ile;
      if($ile==0){
          echo '<p id="error">Brak aktualości w bazie</p>';
      }
      elseif($ile>1){
          echo '<p id="error">Błąd w bazie. Gdy błąd będzie się powtarzał skontaktuj się z administratorem</p>';
      }
      else{
          $tytul=$this->pobierz_dane('tytul',$id);
          $tekst=$this->pobierz_dane('tekst',$id);
          $data=$this->pobierz_dane('data',$id);
          $img=$this->pobierz_dane('img',$id);
          echo'<div id="main_text" style=";margin-top:10px;background-color: white; padding:0px 0px;">';
          echo'<div style="border-bottom:1px solid #6600ff; padding:5px 5px;background:#f4f9fb"><b>'.$tytul.'</b>';
          echo'<div style="font-size:10px; color:silver">Dodano '.$data.'</div></div>';
          echo'<P style="margin:0px;padding:8px 5px">';
          if($img) echo '<img src="'.$img.'" align="left" style="margin-right:5px">';
          echo $tekst.'</p>';
          echo '</div>';
      } 
}
 
function skroc($tekst,$ile)
{
        $znaki = strlen($tekst);
        if ($znaki>=$ile)
        {
                $tnij = substr($tekst,0,strpos($tekst, ".", $ile));
                $skrocone = $tnij.".";
        }
        else
        {
                $skrocone = $tekst;
        }
        return $skrocone;
}
 
function html2txt($document){
  $search = array('@<[\/\!]*?[^<>]*?'.'>@si'
                  );
  $text = preg_replace($search, '', $document);
 
  return $text;
}
 
function show_news_zaj($id){
      $sql = "SELECT * FROM rada_news WHERE akcept=1 AND id=$id";
      $result = mysql_query($sql);
      $ile = mysql_num_rows($result);
      $row = mysql_fetch_array($result);
      //echo $ile;
      if($ile==0){
          echo '<p id="error">Brak aktualości w bazie</p>';
      }
      elseif($ile>1){
          echo '<p id="error">Błąd w bazie. Gdy błąd będzie się powtarzał skontaktuj się z administratorem</p>';
      }
      else{
          $tytul=$this->pobierz_dane('tytul',$id);
          $tekst=$this->pobierz_dane('tekst',$id);
          $data=$this->pobierz_dane('data',$id);
          $img=$this->pobierz_dane('img',$id);
          echo'<table><tr><td><div id="main_text" style=";margin-top:10px;background-color: white; padding:0px 0px;">';
          echo'<div style="border-bottom:1px solid #3b8cdd; padding:5px 5px;background:#f4f9fb"><b><a href="'.WWW.'aktualnosc/'.$row['id'].',_">'.$tytul.'</a></b>';
          echo'<div style="font-size:10px; color:silver">Dodano '.$data.'</div></div>';
          echo'<P style="margin:0px;padding:8px 5px">';
          if($img) echo '<img src="'.$img.'" align="left" style="margin-right:5px">';
          $tekst = $this->html2txt($tekst);
          $tekst = $this->skroc($tekst,400);
          echo $tekst.'</p>';
          echo '</div></td></tr></table>';
      } 
}
 
}
?>
 
[PHP] pobierz, plaintext 

Czy sądzicie że dobrze zaczynam? Czy to jest programowanie obiektowe, czy tylko pseudo-obiektowe? Co muszę poprawić, na co zwrócić uwagę(IMG:style_emoticons/default/questionmark.gif)

[smentek]

Ok ja tez się dorzucę. Jest to programowanie pseudo obiektowe. Ale bardzo dobrze, że próbujesz nie od razu Rzym zbudowano, z czasem Twój kod będzie dużo lepszy.

Twój kod niestety nie jest kodem obiektowym. I można to stwierdzić to zanim nawet człowiek spojrzy na pierwszą jego linijkę. Dobry obiekt ma bardzo wąska odpowiedzialnośc a Ty już na wstępie piszesz:

Napisałem sobie pierwszą klasę, która ma być odpowiedzialna za obsługę artykułów (tworzenie newsów, kategorii, administrację)

Za dużo odpowiedzialności, zresztą wyimaginowanej bo Twój obiekt w najlepszym razie odpowiada za dostęp do danych i za nic więcej. Jest to nieprzemyślana hybryda Active Record z wmieszanymi elementami prezentacji danych. Fuj! (IMG:style_emoticons/default/smile.gif)

Zainteresuj się MVC, a póki co, może zrób z tego co masz dwa obiekty niech jeden pobiera dane a drugi wyświetla dane które pobrał pierwszy.

I jeszcze uwaga:
Crozin:

Jeżeli jakaś metoda nie może zakończyć się poprawnie powinna wyrzucić wyjątek.

To nie jest prawda... Prawdziwsze jest stwierdzenie:

Jeżeli wykonywanie jakiejś metody może zakończyć się błędem, przy czym jest to błąd, nad którym programista piszący dany kod nie ma kontroli (np. kod korzysta z zasobów zewnętrznych, które mogą być niedostępne) to należy wyrzucić wyjątek (obsłużony w innym miejscu programu).

Panie Crozin, warto chyba ubrać to co się chce powiedzieć w parę zdań tak aby to miało ręce i nogi, a nie rzucać sentencje w tylu "Jak się jest głodnym to trzeba zjeść"...