OOP PHP 新手,需要对第一个 Geo RSS 类进行评论
我对 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 技术交流群。
绑定邮箱获取回复消息
由于您还没有绑定你的真实邮箱,如果其他用户或者作者回复了您的评论,将不能在第一时间通知您!
发布评论
评论(4)
公共
或私有
,则属性应始终受到保护
。protected $items
,在getFeed
中缺少$items = ''
>。__construct
中的$this->items = array();
。不要管理自己的
item_count
。更好地依赖 PHP 自己的数组附加功能:好多了,不是吗?
不要声明超出您需要的变量:
这里你不需要数组键。因此,不要在
foreach
循环中获取它;)而是使用$item
作为值,这比$item_element
更好。protected
if there isn't a compelling reason to make thempublic
orprivate
.protected $items
in the class body and a$items = ''
ingetFeed
.$this->items = array();
in__construct
.Don't manage an own
item_count
. Better rely on PHP's own array appending facilities:Much nicer, isn't it?
Don't declare more variables then you need:
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
.这里有几点:
setItem
中的数组)时,就表明您正在使用另一个对象。因此,创建一个class RSSItem
或类似的东西,将这些东西作为成员。如果我想到更多,我会编辑我的答案。
Here are some points:
setItem
, it indicates you have another object in play. So make aclass RSSItem
or something like that, with those things as members.If I think of more I'll edit my answer.
我对这个类的唯一疑虑是在你的
setItem
函数中,你应该只使用[]
表示法来推送这样的关联数组:这样你就可以放弃你的 < 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:This way you can ditch your
$item_count
index variable.Also, letting your properties be
public
is usually a Very Bad Idea.对于第一次来的人来说看起来不错!您在哪里处理作为参数发送的数据?就我个人而言,我会使用类方法处理所有内容,对象的目的是包含对象。这意味着与它们相关的所有处理都应该发生在类本身内部。
另外,也许使用继承和公共、私有成员、使用其他类的类是一个好主意,就像 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.