A few observations on the code:
(1) A consistent indentation style would go a long way toward making the code more readable. For example:
sub new { my ($class, %args) = @_; return 'Error in syntax usage' unless _arg_validation(\%args); my $self = { ua => _mk_ua(), login => $args{login}, password => $args{password}, dir => $args{dir}, threads => 30, }; bless $self, $class; die 'ERROR WITH CONNECTION AUTH' unless $self->_connection_try(); return $self; }
(2) A consistent exception-handling strategy would make it much easier for client code to handle runtime errors in the mp3vk_api module. The constructor, for example, currently throws an exception if the attempt to establish a connection fails, but returns a string if the arguments are invalid. The search method returns a different string on possible error. If the driver script needs to deal with these errors gracefully, the logic quickly becomes messy:
my $a; eval { my $vk = mp3vk_api->new ( login => 'mail@mail.com', password => 'passwd', dir => 'Rhapsody', threads => '10', ); if (ref($vk) eq 'mp3vk_api') { my $out = $vk->search('Rhapsody'); if (ref $out) { my $a = $vk->vk_download($out); } else { # handle possible search failure # throw an exception } } else { # handle constructor failure # throw an exception } }; if ($@) { # catch all exceptions and handle those which haven't been handled + already } else { # use $a normally }
Much simpler to just die (throw an exception) on every exceptional condition.
(3) The number of threads is specified as '10' when the constructor is called, but then overriden as 30 in the constructor itself.
(4) There are many opportunities for simplifying the code’s logic. As one example, here is my re-write of sub _arg_validation:
sub _arg_validation { my $args = shift; my $dir = "$ENV{HOME}/vk_music/"; $dir .= "$args->{dir}/" if defined $args->{dir}; mkdir $dir; $args->{dir} = $dir; return 1 if defined $args->{login} && defined $args->{password} && $args->{login} =~ /^.*?\@.*?$/ && $args->{password} =~ /\w{2,}/; return 0; }
Hope that helps,
| Athanasius <°(((>< contra mundum | Iustus alius egestas vitae, eros Piratica, |
In reply to Re: mp3 search and download
by Athanasius
in thread mp3 search and download
by builat
| For: | Use: | ||
| & | & | ||
| < | < | ||
| > | > | ||
| [ | [ | ||
| ] | ] |