OOP PHP 新手,需要对第一个 Geo RSS 类进行评论

发布于 2024-09-29 00:07:34 字数 2401 浏览 1 评论 0原文

我对 OOP PHP 完全陌生,目前正在阅读“PHP 对象、模式和实践”。我需要开发一些可以生成 GeoRSS 提要的东西。这就是我所拥有的(它工作完美,我只是想对我可以做哪些不同/更有效/更安全的事情进行一些批评):

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;
 }
}

I'm completely new to OOP PHP and currently reading "PHP Objects, Patterns and Practice". I needed to develop something that will generate a GeoRSS feed. This is what I have (it works perfectly, I would just like some critique as to what I could do different / 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;
 }
}

如果你对这篇内容有疑问,欢迎到本站社区发帖提问 参与讨论,获取更多帮助,或者扫码二维码加入 Web 技术交流群。

扫码二维码加入Web技术交流群

发布评论

需要 登录 才能够评论, 你可以免费 注册 一个本站的账号。

评论(4

胡渣熟男 2024-10-06 00:07:34
  1. 如果没有令人信服的理由将属性设置为公共私有,则属性应始终受到保护
  2. 在使用所有变量之前声明或初始化它们:您在类主体中缺少 protected $items ,在 getFeed 中缺少 $items = '' >。
  3. 正确初始化变量:__construct 中的$this->items = array();
  4. 不要管理自己的item_count。更好地依赖 PHP 自己的数组附加功能:

    $this->items[] = array(
        'pubDate' =>;日期(“D,j MYH:i:s T”,$ item_pubDate),
        '标题' => $项目标题,
        '链接' => $item_link,
        '描述' => $item_description,
        '地理:纬度' => $item_geolat,
        '地理:长' => $item_geolong,
        'georss:点' => $item_geolat。" ".$item_geolong,
    );
    

    好多了,不是吗?

  5. 不要声明超出您需要的变量:

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

    这里你不需要数组键。因此,不要在 foreach 循环中获取它;)而是使用 $item 作为值,这比 $item_element 更好。

  1. Properties shall always be protected if there isn't a compelling reason to make them public or private.
  2. Declare or initiate all variables before you use them: You are missing a protected $items in the class body and a $items = '' in getFeed.
  3. Initiate the variables correctly: $this->items = array(); in __construct.
  4. Don't manage an own item_count. Better rely on PHP's own array appending facilities:

    $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,
    );
    

    Much nicer, isn't it?

  5. Don't declare more variables then you need:

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

    Here you didn't need the array key. So don't fetch it in the foreach loop ;) Instead use $item for the value, which is better then $item_element.

绅刃 2024-10-06 00:07:34

这里有几点:

  1. 为什么所有成员都是公开的?您可以在构造函数中设置它们,然后使用它们来构建提要。所以让任何人随意改变它们可能不是一个好主意。无论如何,它们不应该是每个实例的最终/不变吗?
  2. 您的项目可能应该是一个单独的类。每当您看到您正在创建一个大的关联数组(如 setItem 中的数组)时,就表明您正在使用另一个对象。因此,创建一个 class RSSItem 或类似的东西,将这些东西作为成员。

如果我想到更多,我会编辑我的答案。

Here are some points:

  1. Why are all the members public? You set them in the constructor, and then use them to build the feed. So it's probably not a good idea to let anyone change them at will. Shouldn't they be final / unchanging for each instance anyway?
  2. Your items should probably be a separate class. Whenever you see that you're creating a big associative array like you have there in setItem, it indicates you have another object in play. So make a class RSSItem or something like that, with those things as members.

If I think of more I'll edit my answer.

东风软 2024-10-06 00:07:34

我对这个类的唯一疑虑是在你的 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);
 }

这样你就可以放弃你的 < code>$item_count 索引变量。

此外,让您的属性公开通常是一个非常糟糕的主意。

The only qualm I have with this class is in your setItem function, you should just use [] notation to push an associative array like this:

 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);
 }

This way you can ditch your $item_count index variable.

Also, letting your properties be public is usually a Very Bad Idea.

风轻花落早 2024-10-06 00:07:34

对于第一次来的人来说看起来不错!您在哪里处理作为参数发送的数据?就我个人而言,我会使用类方法处理所有内容,对象的目的是包含对象。这意味着与它们相关的所有处理都应该发生在类本身内部。

另外,也许使用继承和公共、私有成员、使用其他类的类是一个好主意,就像 Tesserex 建议的那样。不过,OOP 的开始还是不错的。

Looks good for a first timer! Where do you process the data that you send as parameters? Personally, I would process everything withing the class methods, the purpose of objects is to contain, well, objects. That means that all the processing related to them should happen inside the class itself.

Also, maybe it's a good ideea to play with inheritance and public, private members, classes that use other classes, like Tesserex suggested. Still, nice start on OOP there.

~没有更多了~
我们使用 Cookies 和其他技术来定制您的体验包括您的登录状态等。通过阅读我们的 隐私政策 了解更多相关信息。 单击 接受 或继续使用网站,即表示您同意使用 Cookies 和您的相关数据。
原文