Re: [Bug-gnupod] mktunes.pl creates corrupt iTunesDB ?

[H. Langos]

Hi Richard,

On Wed, Jun 17, 2009 at 10:00:16PM +0200, Richard van den Berg wrote:
> On 6/17/09 10:41 AM, H. Langos wrote:
>>> -   $mktunes->WriteItunesDB;
>>> +   $mktunes->WriteItunesDB(Keep=>$opts{'keepattr'});
>>>     
>>
>> Shouldn't that be :
>>
>> $mktunes->WriteItunesDB( {Keep => $opts{'keepattr'}} );
>>   
>
> It turns out it depends on your usage. The form I chose you can used as:
>
> ($self,%args) = @_;
> @keep=split(/[ ,]+/, $args{Keep});
>

This form passes the whole hash as a key/value list and creates a new one in
the sub. It will break if you add other parameters as the "receiving" %args
hash will try to suck up all he parameters that you pass.

$mktunes->WriteItunesDB(Keep=>$opts{'keepattr'},"foo","bar"); 
will let foo and bar end up in %args:

You might try to separate them but 

($self,%args,$localfoo,$localbar) = @_;
will end up
%args == ( Keep => value of $opts{'keepattr'},
          foo  => bar )
$localfoo == undef
$localbar == undef

> While the form with the {} can be used as:
>
> ($self,$args) = @_;
> @keep=split(/[ ,]+/, $args->{Keep});


This form only passed a single hash reference.

thus 
($self,$args,$localfoo,$localbar) = @_;
will assign a single scalar to $args and $localfoo and $localbar will
get their values.

Did I mention before that I hate perl? :-)

>
> Both forms are in use in the gnupod code at the moment, I just picked  
> the first..

Check if they are used for the same purpose. :-)

> Here is a new patch with your suggestions and the tunes2pod code. The  
> diff for XMLhelper.pm looks a bit weird, but all I did was move the  
> reset code to a new resetxml() sub. The reset is needed because the  
> playlists are stored in XDAT by doxml() and won't be overwritten by the  
> playlists from the iTunesDB otherwise.

I'll take a look at a revised patch later ... got to hurry to work.

-henrik