Bot for navigating a French dating site

I've been using Perl for the last two years and never really had any code to compare it to.

Sometimes I hear people say things like "your code is awful" or "you just write C code with bad performance". I'm willing to improve myself as a Perl programmer. What should I do in order to reach this goal?

This Perl script is a bot to navigate adopteunmec.fr, a French dating site.

Some precisions:

Gogole is the search engine of adopteunmec.fr, AUM::Config is a Perl module containing a hash with credentials, keywords for gogole and path to save file.

(Gogole is the name, not a typo, adopteunmec.fr gave to their search utility. It's a pun (gogole is an abbreviation for mongolien, which is a person afflicted by down syndrome).)

```
use strict;
use warnings;
use IO::File;
use WWW::Mechanize::Firefox;
use AUM::Config;

$ENV{ PERL_LWP_SSL_VERIFY_HOSTNAME } = 0;
$SIG{ INT } = \&clean_disconnect;

my %cfg = AUM::Config->get_cfg;
my $mech = WWW::Mechanize::Firefox->new( launch => 'firefox' );
my $url = 'https://www.adopteunmec.com';
my $bait = 0;
my $res;

sub clean_disconnect {
print "Going to disconnect you now...\n";
$mech->get( 'http://www.adopteunmec.com/auth/logout' );
print "Done ! Good-bye\n";
undef $mech;
exit 0;
}

sub connect_and_fetch {
print "Connecting to your profile...\n";
$mech->get( $url );
$mech->field( '#mail', $cfg{ auth }{ user_name } );
$mech->field( '#password', $cfg{ auth }{ passwd } );
$mech->click_button( value => 'OK' );
$url = $mech->uri;
$mech->get( $url );

if ( $url eq 'https://www.adopteunmec.com/index?e=login' ) {
print STDERR "connection failed, wrong username or password or already logged in";
return undef;
}
print "Connected\n";

print "Fetching home page\n";
$res = $mech->content;
print "Fetched\n";

return 1;
}

sub get_link_home_page {
my @link_match_home = $res =~ /https?:\/\/www\.adopteunmec\.com\/profile\/[0-9]+/g;
return @link_match_ho

Generally

First, do not get put down by negative feedback like 'this code is a mess and it sucks'. Be proud of what you have developed and evolve. Save it, and look back at it in a few years. The most important thing about writing code, is solving problems at hand.

The book "Perl best practices" might be a good place to start. Also you can use the module Perl::Critic to "enforce" these practices on your code.

However, many best practices are a matter of opinion - and frankly I personally do not follow all of them. After reading your code, these are the things I thought could be done differently in addition to the comments from mpapec.

Thou shalt not use globals

This is easier to understand, and safer:

my $foo = 1;
$foo = magic($foo);
printf("%d\n", $foo);

Than this:

my $foo = 1;
magic();
printf("%d\n", $foo);

sub magic {
    $foo++;
}

Please refer to this answer. for more details on "thou shalt not use globals".

Be literal

This is easier to understand:

sub incr {
    my $number = shift || return 0;
    return int($number) ? $number++ : 0;
}

Than this:

sub incr {
    return int($_[0]) ? $_[0]++ : 0;
}

Document your methods using POD

Documentation makes life easy for other people using or handling your code. I've also experienced forgetting what my own code did, and was thankful for remembering to document the methods.

=head2 mymethod

$int = mymethod($integer);

This method increments an integer or returns 0.

=cut

sub mymethod {
    my $interger = shift || return 0;
    return int($_[0])++;
}

See the POD manual for details, perldoc perlpod or online.

Configuration does not belong in a sub routine

As a general guideline, you always reuse code. So when you are developing try to make your methods generic to support reuse, instead of re-invention or copy-paste. That way you will become more efficient over time. Be careful not to over complicate matters, the most important thing is to actually writing code and solving the problem at hand. I've done this mistake myself. Often I will refactor my code after solving the original problem to support future re-use.

Bad:

my $stuff = dostuff();
sub dostuff {
    my $url = 'http://loveshack.com';
    return connect($url);   
}

Good:

my $stuff = dostuff('http://loveshack.com');
sub dostuff {
    my $url = shift || return;
    return connect($url);
}

Even better, you can support overriding these thing by using Getopt::Long or a configuration framework. Do not over complicate this. If your "run script" is small and very specific to the usage, I would just do it inline like in the example above.

Separate "usage" from "application" logic

This usually involves taking your methods, making them generic (ie no "use specific" configuration inside the subs) and put them inside a separate package. Be it object oriented or not. perldoc perlmod or perldoc perlootut is your friend. There's also loads of articles online on these topics. Personally looking to other peoples code for example helped me in this direction, mostly on CPAN.

Use a logger

Log::Log4perl is a good place to start. Actually it can make your life much simpler because you can flip the output by tuning the log configuration. That way your code can always provide debug output, and you can simply choose to ignore it by configuration in your "run script".

Write tests

Once you have separated your "run specific logic" from the "application logic", you could write tests using Test::Simple or Test::More. Why write tests? To avoid manual testing and bugs after altering code.

Useful resource for writing your first tests.

Making a module

When you decide to take your code and make a module of it - and write tests - you should use the Module::Maker module for structure. You don't have to publish something on CPAN just because you made a solid module of your code, but at least then you can if you decide to do so.

StackExchange Code Review Q#73320, answer score: 8