Re: [Bug-gnupod] mktunes.pl creates corrupt iTunesDB ?
Hi Richard,
On Fri, Jun 19, 2009 at 08:26:30PM +0200, Richard van den Berg wrote:
On 6/19/09 11:07 AM, H. Langos wrote:
Maybe the merge code should only be active if your memory saving feature is
active?
Here is the new patch that does just that. I love git, it's super fast. :-)
One more question:
Why did you use
resetxml;
instead of
resetxml();
?
I know the former doesn't pass an empty @_ array for the called sub but
passes the existing argument list. But you don't do anything with @_ in
resetxml(). So why bother passing the current arguments list on to it?
cheers
-henrik
diff --git a/src/ext/XMLhelper.pm b/src/ext/XMLhelper.pm
index 748ce22..bb7c88b 100755
--- a/src/ext/XMLhelper.pm
+++ b/src/ext/XMLhelper.pm
@@ -301,19 +301,23 @@ sub mkh {
}
-
#
-# Parses the XML File and do events
-sub doxml {
- my($xmlin, %opts) = @_;
- return undef unless (-r $xmlin);
- ### reset some stuff if we do a second run
+# Reset some stuff if we do a second run
+sub resetxml {
$cpn = undef; #Current PlaylistName
@idpub = ();
@plorder = ();
$xid = 1;
$XDAT = undef;
- ###
+}
+
+
+#
+# Parses the XML File and do events
+sub doxml {
+ my($xmlin, %opts) = @_;
+ return undef unless (-r $xmlin);
+ resetxml;
my $p;
my $ref = eval {
$p = new XML::Parser(ErrorContext = 0,
Handlers={Start=\eventer});
___
Bug-gnupod mailing list
Bug-gnupod@nongnu.org
http://lists.nongnu.org/mailman/listinfo/bug-gnupod
Re: [Bug-gnupod] mktunes.pl creates corrupt iTunesDB ?
H. Langos wrote: One more question: Why did you use resetxml; instead of resetxml(); ? I know the former doesn't pass an empty @_ array for the called sub but passes the existing argument list. But you don't do anything with @_ in resetxml(). So why bother passing the current arguments list on to it? I imagine that the author didn't intend that effect of calling the subroutine that way. I suspect it's a matter of laziness. FWIW, resetxml(); or resetxml(@_); # if necessary are both many, many times preferable to: resetxml; and the latter is generally shunned by experienced, community-minded Perl folk. I'd suggest applying the patch with this tidied up as appropriate. J ___ Bug-gnupod mailing list Bug-gnupod@nongnu.org http://lists.nongnu.org/mailman/listinfo/bug-gnupod
Re: [Bug-gnupod] mktunes.pl creates corrupt iTunesDB ?
On Wed, July 1, 2009 16:13, Jacinta Richardson wrote: I imagine that the author didn't intend that effect of calling the subroutine that way. Absolutely right. I suspect it's a matter of laziness. I claim ignorance. I didn't know there was a fundamental difference between the two ways of calling a subroutine. Thanks for the explanation. I'd suggest applying the patch with this tidied up as appropriate. I agree. Cheers, Richard (learning all the time) ___ Bug-gnupod mailing list Bug-gnupod@nongnu.org http://lists.nongnu.org/mailman/listinfo/bug-gnupod
Re: [Bug-gnupod] mktunes.pl creates corrupt iTunesDB ?
H. Langos wrote:
Speaking of readability and code style. Is there a consensus among the
experienced and community-minded folks in regard to the passing of
parameters to subs? Especially when writing subs in modules?
I nowadays prefer to pass parameters as one hashref
subname({foo = x, bar = y});
instead of as a naked list:
subname(x, y);
or (heaven help)
subname(foo = x, bar = y);
which looks like a hash but is only a list.
I took a look at perlstyle(1) and perlsub(1) but I didn't see any hints in
regard to this.
This practice is called named arguments and is a great way of calling
subroutines because it removes the confusion of which order the arguments need
to go in. It also allows you to set and use defaults, for example:
subname({foo = x, bar = y});
subname({foo = x});
subname({bar = y});
subname();
...
my %defaults = (
foo = 'a',
bar = 'b',
};
sub subname {
my ($args_ref) = @_;
state %defaults = ( # 5.10+ only, else my %defaults
foo = 'a', # or a closure
bar = 'b',
);
$args_ref = { %defaults, %{$args_ref} };
# Now you know for sure that foo and bar have values
# for all the above calls.
}
It's not discussed in perlstyle, and perlsub because the maintainers of those
files haven't really decided on a one-true way, or because they've been too lazy
to write it... Damian Conway wrote bunch of good ideas about good Perl style in
his book called Perl Best Practices, which is what most people point to when
they talk about Perl style now. These practices have been turned into
requirements by the Perl::Critic system too, so you can get automated feedback
on your code quality by using the Perl::Critic programs.
Anyway, Damian advises:
Use a hash of named arguments for any subroutine that has more than 3
parameters
and agrees entirely with your preference:
subname({foo = x, bar = y});
rather than the alternatives you give.
This has the advantage that mistakes in how you call the subroutine may be
spotted at compile time rather than run time:
subname({foo = x, bar = y, cols = 1..4});
Odd number of elements in anonymous hash at demo.pl line 2
He also says it's okay to mix positional and named arguments if you don't have
many but some are always mandatory. For example:
padded($string, {cols = 20, centered =1, filler=$SPACE});
sub padded {
my ($text, $args_ref) = @_;
# pad the string.
}
Hope this helps.
J
--
(`-''-/).___..--''`-._ | Jacinta Richardson |
`6_ 6 ) `-. ( ).`-.__.`) | Perl Training Australia|
(_Y_.)' ._ ) `._ `. ``-..-' | +61 3 9354 6001|
_..`--'_..-_/ /--'_.' ,' | cont...@perltraining.com.au |
(il),-'' (li),' ((!.-' | www.perltraining.com.au |
___
Bug-gnupod mailing list
Bug-gnupod@nongnu.org
http://lists.nongnu.org/mailman/listinfo/bug-gnupod
Re: [Bug-gnupod] artwork support apparently (but not really) broken on ubuntu
Hello Henrik, The behaviour is not intended as you so nicely put it. It is/was a bug and has been corrected in the 6.4.9-9 version. From the ChangeLog: 2009-03-01 6.4.9-9 Cristy quetzlzacatena...@image... * Convert returns MagickFalse for the -version option (reference http://www.imagemagick.org/discourse-server/viewtopic.php?f=2t=13230). I think conversion -version is a correct way to check whether ImageMagick is installed. autoconf contains a macro AC_PATH_PROG that checks the existence of the executable But there is no predefined macro as far as I can see that checks whether a particular version is installed However I haven't been able to figure out what to do with the IMAGIC variable. It seems indeed unused elsewhere. I'll try to figure that out next a+ chriss ___ Bug-gnupod mailing list Bug-gnupod@nongnu.org http://lists.nongnu.org/mailman/listinfo/bug-gnupod
