builat has asked for the wisdom of the Perl Monks concerning the following question:

Good day collective mind. Let me show you my ugly code for search and download mp3 from social network vk.com If some one could show me the better way it will be realy good.
#!/usr/bin/perl use warnings; use strict; use 5.018; use mp3vk_api; use utf8; my $vk = mp3vk_api->new( login => 'mail@mail.com', password => 'passwd', dir =>'Rhapsody', threads=>'10', ); my $out = $vk->search('Rhapsody'); my $a = $vk->vk_download($out);
and pm.
#!/usr/bin/perl package mp3vk_api; use strict; use warnings; use Env qw(HOME); use utf8; use Encode; use URI::Escape; use HTML::Entities; use LWP; use LWP::Protocol::https; use LWP::Simple; use Parallel::ForkManager; our $VERSION = 0.2; #constructor sub new { my ($class, %args) = @_; return ('Error in syntax usage') if(_arg_validation(\%args) != 1) +; my $self = { ua => _mk_ua(), login => $args{login}, password => $args{password}, dir =>$args{dir}, threads=>30, }; bless $self, $class; die ('ERROR WITH CONNECTION AUTH') if($self->_connection_try() != 1) +; return $self; } #Creation of user agent. Private method sub _mk_ua { my $ua = LWP::UserAgent->new(); push @{ $ua->requests_redirectable }, 'POST'; $ua->cookie_jar( {} ); return ($ua); } #public method. search via vk. sub search { my ($self, $query) = @_; my $res = $self->{ua}->get('http://vk.com/search?c[section]=audio&c[ +q]='.uri_escape_utf8($query)); eval{$res->is_success;} or return 'Error possible with LWP_ua see er +r msg -> '.$res->status_line; # my @findnings = $res->decoded_content =~ m'<input type="hi +dden" id="audio_info(.*?)</table>'sgi; my @findnings = $res->decoded_content =~ m'<div id="audio.* +?>(.*?)<div class="ai_controls">'sgi; my @dump; push @dump, $self->_cleanup_response($_) for(@ +findnings); @dump = grep { defined $_ } @dump; return (\@dump); } #parsing of vk response. private method sub _cleanup_response { my ($self, $str) = @_; my ($title) = $str =~ m{<span class="ai_title">(.*?)</span>}si; return undef unless $title; $title =~ s/(<.*?>|\(.+\)|\&\w+|[;'"])//g; $title =~ s/\s+/ /g; $title = decode_entities($title); my ($duration) = $str =~ m{<div class="ai_dur" data-dur="\d+" onclic +k="audioplayer.switchTimeFormat(this, event);">(\d+:\d+)</div>}i; my ($link) = $str =~ m{value="(http://[^'"`]+\.mp3)}i; $duration = 0 unless($duration); return { title => $title, duration => $duration, link => $link }; } #check posibility of connection sub _connection_try{ my $self = shift; my $res = $self->{ua}->post('https://login.vk.com/?act=login', { email => $self->{login}, pass => $self->{password}, }); if( $res->is_success && ($res->decoded_content =~ /var\s+vk\s*=\s*\{[^\{\}]*?id\s*\:\s*( +\d+)/i || $res->decoded_content =~ m#login\.vk\.com/\?act=logout#i ) ) + { $self->{id} = $1; return 1; } return 0; } #validation of login and passwd sub _arg_validation { my $args = shift; unless (defined $args->{dir}){ $args->{dir} = "$ENV{HOME}/vk_music/"; mkdir $args->{dir}; } else{ $args->{dir} = "$ENV{HOME}/vk_music/$args->{dir}/"; mkdir $args->{dir}; } unless(defined $args->{login} && defined $args->{password} && $args->{login} =~ /^.*?\@.*?$/ && $args->{ +password} =~ /\w{2,}/){ return 0; } else{ return 1; } } sub vk_download{ my $self = shift; my $ref_dump = shift; my $pm = Parallel::ForkManager->new($self->{threads}); foreach (@{$ref_dump}) { $pm->start and next +; # do the fork if (getstore($_->{ +link}, "$self->{dir}/$_->{title}.mp3") != RC_OK){ prin +t 'error occured '."\n"; } else{ print + $_->{title}.'.................. downloaded'."\n"; } $pm->finish; # do th +e exit in the child process } } 1;

Replies are listed 'Best First'.
Re: mp3 search and download
by Athanasius (Archbishop) on Mar 08, 2014 at 15:40 UTC

    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,

Re: mp3 search and download
by aitap (Curate) on Mar 08, 2014 at 19:18 UTC

    Pasring HTML manually is not usually a good idea because it may break if site design is changed.

    VKontakte social network has an official API. You can try calling API methods (get "https://api.vk.com/method/audio.search?...";) and decoding JSON yourself or use a module (for example, Net::VKontakte::Standalone which is a thin wrapper around JSON and WWW::Mechanize developed by me).

    Registering an application can be avoided by passing any valid APP_ID (integer number). Methods requiring application secret to work won't work this way.