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

Title:
Use:  <p> text here (a paragraph) </p>
and:  <code> code here </code>
to format your post, it's "PerlMonks-approved HTML":



  • Posts are HTML formatted. Put <p> </p> tags around your paragraphs. Put <code> </code> tags around your code and data!
  • Titles consisting of a single word are discouraged, and in most cases are disallowed outright.
  • Read Where should I post X? if you're not absolutely sure you're posting in the right place.
  • Please read these before you post! —
  • Posts may use any of the Perl Monks Approved HTML tags:
    a, abbr, b, big, blockquote, br, caption, center, col, colgroup, dd, del, details, div, dl, dt, em, font, h1, h2, h3, h4, h5, h6, hr, i, ins, li, ol, p, pre, readmore, small, span, spoiler, strike, strong, sub, summary, sup, table, tbody, td, tfoot, th, thead, tr, tt, u, ul, wbr
  • You may need to use entities for some characters, as follows. (Exception: Within code tags, you can put the characters literally.)
            For:     Use:
    & &amp;
    < &lt;
    > &gt;
    [ &#91;
    ] &#93;
  • Link using PerlMonks shortcuts! What shortcuts can I use for linking?
  • See Writeup Formatting Tips and other pages linked from there for more info.