New to OOP PHP, you need to criticize Geo RSS first class

Question

I am completely new to OOP PHP and am currently reading "PHP Objects, Patterns and Practice". I needed to develop something that would create the GeoRSS channel. This is what I have (it works great, I would just like me to criticize what I can do to others / more efficiently / safer):

class RSS {
 public $channel_title;
 public $channel_description;
 public $channel_link;
 public $channel_copyright;
 public $channel_lang;
 public $item_count;
 public function __construct ($channel_title, $channel_description, $channel_link, $channel_copyright, $channel_lang) {
  $this->channel_title = $channel_title;
  $this->channel_description = $channel_description;
  $this->channel_link = $channel_link;
  $this->channel_copyright = $channel_copyright;
  $this->channel_lang = $channel_lang;
  $this->items = "";
  $this->item_count = 0;
    }
 public function setItem ($item_pubDate, $item_title, $item_link, $item_description, $item_geolat, $item_geolong) {
  $this->items[$this->item_count]['pubDate'] = date("D, j M Y H:i:s T",$item_pubDate);
  $this->items[$this->item_count]['title'] = $item_title;
  $this->items[$this->item_count]['link'] = $item_link;
  $this->items[$this->item_count]['description'] = $item_description;
  $this->items[$this->item_count]['geo:lat'] = $item_geolat;
  $this->items[$this->item_count]['geo:long'] = $item_geolong;
  $this->items[$this->item_count]['georss:point'] = $item_geolat." ".$item_geolong;
  $this->item_count++;
 }
 public function getFeed () {
  foreach ($this->items as $item => $item_element) {
   $items .= "    <item>\n";
   foreach ($item_element as $element => $value) {
    $items .= "      <$element>$value</$element>\n";
   }
   $items .= "    </item>\n";
  }
  $feed = "<?xml version=\"1.0\" encoding=\"utf-8\"?>\n"
    . "<rss version=\"2.0\" xmlns:geo=\"http://www.w3.org/2003/01/geo/wgs84_pos#\" xmlns:georss=\"http://www.georss.org/georss\">\n"
    . "  <channel>\n"
    . "    <title>".$this->channel_title."</title>\n"
    . "    <description>".$this->channel_description."</description>\n"
    . "    <link>".$this->channel_link."</link>\n"
    . "    <copyright>Copyright ".date("Y")." ".$this->channel_copyright.". All rights reserved.</copyright>\n"
    . "    <lang>".$this->channel_lang."</lang>\n"
    . $items
    . "  </channel>\n"
    . "</rss>";
  return $feed;
 }
}

Answer 1

• Properties should always be protected, if there is no good reason to make them publicor private.
• Declare or start all variables before using them: you are missing protected $itemsin the body of the class and $items = ''in getFeed.
• : $this->items = array(); __construct.

• item_count. PHP:

  $this->items[] = array(
      'pubDate'      => date("D, j M Y H:i:s T",$item_pubDate),
      'title'        => $item_title,
      'link'         => $item_link,
      'description'  => $item_description,
      'geo:lat'      => $item_geolat,
      'geo:long'     => $item_geolong,
      'georss:point' => $item_geolat." ".$item_geolong,
  );
  

  , ?

• , :

  foreach ($this->items as $item) {
      $items .= "    <item>\n";
      foreach ($item as $element => $value) {
           $items .= "      <$element>$value</$element>\n";
      }
      $items .= "    </item>\n";
  }
  

  . foreach;) $item , , $item_element.

Answer 2

:

• ? , . , , . / ?
• , , . , , , setItem, , . , class RSSItem - , .

, .

Answer 3

, , setItem, [] :

 public function setItem ($item_pubDate, $item_title, $item_link, $item_description, $item_geolat, $item_geolong) {
  $this->items[] = array(
                         'pubDate' => date("D, j M Y H:i:s T",$item_pubDate),
                         'title' => $item_title,
                         'link' => $item_link,
                         'description' => $item_description,
                         'geo:lat' => $item_geolat,
                         'geo:long' => $item_geolong,
                         'georss:point' => $item_geolat.' '.$item_geolong);
 }

$item_count.

, public .

Answer 4

Looks good for the first timer! Where do you process the data you send as parameters? Personally, I handled everything using class methods, the purpose of objects is to contain, well, objects. This means that all processing associated with them must occur within the class itself.

Also, it might be a good idea to play with inheritance and public, private members, classes that use other classes, as Tesserex suggested. However, it's nice to start with OOP.