Skip Menu | You are currently an anonymous guest. | Login | Return to Main | About rt.cpan.org
 

Please report any issues with rt.cpan.org to rt-cpan-admin@bestpractical.com.

X Report information
Id: 34914
Status: resolved
Worked: 30 min
Left: 0 min
Priority: 0/0
Queue: XML-RSS

Owner: SHLOMIF <SHLOMIF [...] cpan.org>
Requestors: ARTY <ARTY [...] cpan.org>
Cc:
AdminCc:

Severity: Wishlist
Broken in: (no value)
Fixed in: (no value)




X History Display mode: Brief headersFull headers
#   Sat Apr 12 09:34:50 2008 ARTY - Ticket created  
Subject: Suggestion: make 'parse' and 'parsefile' methods to be callable as class methods
[text/plain 604b]
Hi guys!

It would be convenient to make 'parse' and 'parsefile' methods
to be callable as class methods.

Say, I want to parse RSS file. I have to do something like this.
my $rss = XML::RSS->new();
$rss->parsefile('file.rss');

It would be handy to do one action instead of two.
my $rss = XML::RSS->parsefile('file.rss');

There is one issue. Now both methods return RSS version number.
This behavior isn't documented and seems to be a side effect of
internal version assignment so I guess it would be nice to always
return $self.

Patch is attached.
Of course it doesn't break backwards compatibility.

Subject: RSS.pm.diff

[text/x-patch 974b]
Index: libxml-rss-perl/lib/XML/RSS.pm
===================================================================
--- libxml-rss-perl/lib/XML/RSS.pm (revision 11067)
+++ libxml-rss-perl/lib/XML/RSS.pm (working copy)
@@ -976,6 +976,8 @@
my $self = shift;
my $text_to_parse = shift;

+ $self = $self->new() unless ref $self;
+
$self->_reset;

# Workaround to make sure that if we were defined with version => "2.0"
@@ -988,11 +990,15 @@

$self->_auto_add_modules if $AUTO_ADD;
$self->{version} = $self->{_internal}->{version};
+
+ return $self;
}

sub parsefile {
my $self = shift;

+ $self = $self->new() unless ref $self;
+
$self->_reset;

# Workaround to make sure that if we were defined with version => "2.0"
@@ -1005,6 +1011,8 @@

$self->_auto_add_modules if $AUTO_ADD;
$self->{version} = $self->{_internal}->{version};
+
+ return $self;
}

# Check if Perl supports the :encoding layer in File I/O.

#   Thu May 22 05:11:55 2008 SHLOMIF - Correspondence added  
[text/plain 1k]
Hi ARTY!

Sorry for the late response.
On Sat Apr 12 09:34:50 2008, ARTY wrote:
> Hi guys!
>
> It would be convenient to make 'parse' and 'parsefile'
methods
> to be callable as class methods.
>
> Say, I want to parse RSS file. I have to do something like
this.
> my $rss = XML::RSS->new();
> $rss->parsefile('file.rss');
>
> It would be handy to do one action instead of two.
> my $rss = XML::RSS->parsefile('file.rss');
>
> There is one issue.

Thanks for the suggestion, but sorry - I don't want to do such a
thing. It adds complexity to the code, and may confuse the user.
Furthermore, you didn't supply an automated test for it.

> Now both methods return RSS version
> number.
> This behavior isn't documented and seems to be a side
effect of
> internal version assignment so I guess it would be nice to
always
> return $self.
>
> Patch is attached.
> Of course it doesn't break backwards compatibility.

This seems ok. (to Return $self, I mean).

Please add an automated test for this. See the files under t/.

Regards,

Shlomi Fish
#   Thu May 22 05:12:06 2008 RT_System - Status changed from 'new' to 'open'  
#   Thu Sep 25 03:52:52 2008 SHLOMIF - Correspondence added 30 min  
[text/plain 156b]
I now implemented the ->parse and ->parsefile returning $self with test
cases in the trunk. The other suggestion will not be implemented.

Thanks!

Closing.
#   Thu Sep 25 03:52:55 2008 SHLOMIF - Status changed from 'open' to 'resolved'  
#   Thu Sep 25 03:52:57 2008 SHLOMIF - Given to SHLOMIF