IdentifiantMot de passe
Loading...
Mot de passe oublié ?Je m'inscris ! (gratuit)
Navigation

Inscrivez-vous gratuitement
pour pouvoir participer, suivre les réponses en temps réel, voter pour les messages, poser vos propres questions et recevoir la newsletter

Langage Perl Discussion :

Revue de code


Sujet :

Langage Perl

  1. #1
    Membre habitué
    Homme Profil pro
    Ingénieur intégration
    Inscrit en
    Mars 2015
    Messages
    138
    Détails du profil
    Informations personnelles :
    Sexe : Homme
    Localisation : France, Morbihan (Bretagne)

    Informations professionnelles :
    Activité : Ingénieur intégration
    Secteur : Service public

    Informations forums :
    Inscription : Mars 2015
    Messages : 138
    Points : 138
    Points
    138
    Par défaut Revue de code
    Bonjour,

    en poste depuis 3 ans au service de la production, il m'a été demandé à mon arrivée de développer des scripts batchs facilement portables sur les différentes plateformes (Aix, GNU/Linux, Microsoft Windows). Les précédents scripts étaient écrits en ksh ou Powershell.

    Ayant de vagues connaissances de Perl, je m'y suis remis grâce aux tutos du site ( à Sylvain Lhullier et aux autres rédacteurs ). Les résultats sont bons et le choix a été officiellement validé.

    Je dois commencer à former quelques personnes sur le langage ; par contre, mon code n'ai jamais été audité et je voulais savoir si cela est possible ici et si oui, de quelle manière (coller le code, pièces-jointes, en privé...).
    J'ai réécrit les jours derniers un modèle de script et 2 modules maisons (entre 100 et 150 lignes pour chaque éléments, lignes blanches incluses).
    Ces éléments s'exécutent correctement, mais je suis intéressé par tout commentaire, suggestion, critique, correction sur les règles de codage, nommage, sémantique, syntaxe utilisés.

    Merci

  2. #2
    Modérateur
    Avatar de kolodz
    Homme Profil pro
    Ingénieur développement logiciels
    Inscrit en
    Avril 2008
    Messages
    2 211
    Détails du profil
    Informations personnelles :
    Sexe : Homme
    Âge : 36
    Localisation : France, Rhône (Rhône Alpes)

    Informations professionnelles :
    Activité : Ingénieur développement logiciels
    Secteur : High Tech - Produits et services télécom et Internet

    Informations forums :
    Inscription : Avril 2008
    Messages : 2 211
    Points : 8 316
    Points
    8 316
    Billets dans le blog
    52
    Par défaut
    Bonjour,

    Si ton code est relativement court, il t'es possible d’utilisé la balise [CODE]. Il est préférable d'éviter les pièces jointes. Peu de personnes iront regarder des pièces jointes.



    Note : Peut-être faire attention, si les script contiennent des informations sensible.

    Cordialement,
    Patrick Kolodziejczyk.
    Si une réponse vous a été utile pensez à
    Si vous avez eu la réponse à votre question, marquez votre discussion
    Pensez aux FAQs et aux tutoriels et cours.

  3. #3
    Rédacteur/Modérateur

    Avatar de Lolo78
    Homme Profil pro
    Conseil - Consultant en systèmes d'information
    Inscrit en
    Mai 2012
    Messages
    3 612
    Détails du profil
    Informations personnelles :
    Sexe : Homme
    Localisation : France, Yvelines (Île de France)

    Informations professionnelles :
    Activité : Conseil - Consultant en systèmes d'information
    Secteur : High Tech - Opérateur de télécommunications

    Informations forums :
    Inscription : Mai 2012
    Messages : 3 612
    Points : 12 469
    Points
    12 469
    Billets dans le blog
    1
    Par défaut
    Oui, poste simplement ton code entre balises de code s'il n'est pas trop long (en anonymisant ce qui doit l'être, le cas échéant).

    On regardera.

    Commence déjà avec un seul script, il y aura peut-être des remarques générale qui s'appliqueront aussi aux autres.

  4. #4
    Membre habitué
    Homme Profil pro
    Ingénieur intégration
    Inscrit en
    Mars 2015
    Messages
    138
    Détails du profil
    Informations personnelles :
    Sexe : Homme
    Localisation : France, Morbihan (Bretagne)

    Informations professionnelles :
    Activité : Ingénieur intégration
    Secteur : Service public

    Informations forums :
    Inscription : Mars 2015
    Messages : 138
    Points : 138
    Points
    138
    Par défaut Revue de code - squelette script principal
    Merci pour votre aide, voici dans un premier temps le modèle de script principal :

    Code : Sélectionner tout - Visualiser dans une fenêtre à part
    1
    2
    3
    4
    5
    6
    7
    8
    9
    10
    11
    12
    13
    14
    15
    16
    17
    18
    19
    20
    21
    22
    23
    24
    25
    26
    27
    28
    29
    30
    31
    32
    33
    34
    35
    36
    37
    38
    39
    40
    41
    42
    43
    44
    45
    46
    47
    48
    49
    50
    51
    52
    53
    54
    55
    56
    57
    58
    59
    60
    61
    62
    63
    64
    65
    66
    67
    68
    69
    70
    71
    72
    73
    74
    75
    76
    77
    78
    79
    80
    81
    82
    83
    84
    85
    86
    87
    88
    89
    90
    91
    92
    93
    94
    95
    96
    97
    98
    99
    100
    101
    102
    103
    104
    105
    106
    107
    108
    109
    110
    111
    112
    113
    114
    115
    116
    117
    118
    119
    120
    121
    122
    123
    124
    125
    126
    127
    128
    129
    130
    131
    132
    133
    134
    135
    136
    137
    138
    139
    140
    141
    142
    143
    144
    145
    146
    147
    148
    149
    150
    151
    152
    153
    154
     
    #!/usr/bin/perl -w
    #============================================================================
    #@(#) Procédure   : skel4.pl
    #@(#) Description : ...
    #@(#)
    #@(#) Pré-requis  : module Perl Shell::Source pour sourcer l'envbatch.
    #@(#)                   initialiser variable PERL5LIB dans ~/.profile pour déclarer
    #@(#)                    le répertoire contenant les modules maison
    #@(#)
    #@(#) Auteur      : xxx
    #@(#) Arguments   :  1 - reprise = balise de début ou ""
    #@(#)                      2 - fin     = balise de fin ou ""
    #@(#)
    #@(#) Historique  : jj/mm/aaaa début écriture programme
    #@(#)               ...
    #============================================================================
    use strict;
    use Cwd;
    use Shell::Source;
    use feature qw(switch say);
    use xxx_utils_v4;
    use oxa_utils_v4;
     
     
     
     
    # initialisation constantes, variables
    use constant h_const => {
        dft_reprise => "debut",
        dft_fin     => "fin",
        dft_nb_args => 2,       # nb arguments par défaut
        nb_args     => 0,       # nb arguments supplémentaires attendus
        domaine     => "PROD",
        application => "TECH",
        na          => "N/A",
    };
     
    our ( $script_absolute_name, $script_relative_name, $script_dir, $script_name, $script_extension, $script_user ) = utils_script($0);
    my $pid = $$;
    my ( $reprise, $fin );
     
    # Source envbatch et envadmin et initialisation variables (envexe sourcé dans .profile)
    my $envbatch = Shell::Source->new(shell => "ksh", file => "$ENV{'PROD_TECH_ENV'}/envbatch");
    $envbatch->inherit;
     
    my  $project        = $ENV{'PROJECT'};
    my  $prod_tech_env  = $ENV{'PROD_TECH_ENV'};
    our $dir_log_oxalys = $ENV{'REP_LOG_OXALYS'};
     
    # Initialisation et création fichier log, puis redirection STDOUT et STDERR
    our ( $basename_log, $log_file ) = utils_log( $script_name, $pid );
    open( my $log, ">", $log_file ) || die "FATAL: cannot open \"$log_file\" for writing: $!\n";
    my $r_log = \*$log;    # référence vers le fichier de log
    select $log;*STDERR = *$r_log;
     
    # Initialisation oxa
    oxa_main(); # pas vraiment utile, juste pour faire comme si ;-)
     
     
     
     
    # Fonctions internes
    sub usage {
        oxa_msg_err( $r_log, 0, "KO - Usage : $script_relative_name <balise de début ou \"\"> <balise de fin ou \"\">",1 );
    }
     
     
    sub controle_balises {
        my ( $balise_debut, $balise_fin, $r_balises_ok ) = @_;
     
        my $flag_err = 0;
     
        foreach my $tag ( $balise_debut, $balise_fin ) {
            if ( ! utils_ctrl_balise( $tag, $r_balises_ok ) ) {
                oxa_msg_warn( $r_log, 1, "KO - Balise $tag incorrecte" );
                $flag_err++;
            }
        }
     
        oxa_msg_err( $r_log, 1, "KO - Balises autorisées : @$r_balises_ok", 1 ) if $flag_err ;
     
        return;
    }
     
     
    sub next_step {
        my ( $step ) = @_;
        if( $reprise eq $fin ) { $reprise = 'fin' } else { $reprise = $step } ;
        return $reprise;
    }
     
     
    sub process {
        my $dummy = 0;
        return;
    }
     
    # main ()
    # - Initialisation et contrôle du nombre d'arguments attendus
    # - Initialisation entêtes OXA
    # - Initialisation variables contenant les arguments
    oxa_debut      ( $r_log, $script_absolute_name );
    oxa_msg_info ( $r_log, 0, "Script de xxxxxx à compléter xxxxxxxxxxxxx" );
    oxa_msg_info ( $r_log, 0, "Fichier de log ... $log_file" );
    oxa_app         ( $r_log, h_const->{domaine}, h_const->{application} );
     
    usage if utils_nb_args() != h_const->{dft_nb_args} + h_const->{nb_args} ;
     
    my $r_balises_ok = ["debut","fin","traitement"];  # réf. anonyme vers tableau des balises autorisées
     
     
    # Tests préliminaires
    # Contrôle balises reçues en arguments
    if ( $ARGV[0] eq "" ) { $reprise = h_const->{dft_reprise} } else { $reprise = $ARGV[0] };
    if ( $ARGV[1] eq "" ) { $fin     = h_const->{dft_fin} }         else { $fin     = $ARGV[1] };
    controle_balises( $reprise, $fin, $r_balises_ok );
     
     
    # Contrôles spécifiques à placer ci-après
     
     
     
     
    # Boucle de traitement principale
    until ( $reprise eq "fin" ) {
        oxa_msg_info( $r_log, 1,"LABEL : $reprise" );
        given ($reprise) {
            when ('debut') {
                oxa_msg_info( $r_log, 1, "===> Début de traitement" );
                next_step("traitement");
            }
            when ('traitement') {
                oxa_msg_info( $r_log, 1, "===> Traitement" );
                process();
                next_step("fin_traitement");
            }
            when ('fin_traitement') {
                oxa_msg_info( $r_log, 1, "===> Fin de traitement" );
                next_step("fin");
            }
            default {
                say "KO - Etiquette $reprise invalide";
                oxa_msg_err( $r_log, 1, "KO - Etiquette $reprise invalide", 99 );
                next_step("fin");
            }
        }
    }
     
     
    # Footer OXA et sortie normale
    oxa_msg_info( $r_log, 1, "--------------------------------------------" );
    oxa_msg_info( $r_log, 1, "                FIN $script_name");
    oxa_fin( $r_log, 0 );

  5. #5
    Modérateur
    Avatar de kolodz
    Homme Profil pro
    Ingénieur développement logiciels
    Inscrit en
    Avril 2008
    Messages
    2 211
    Détails du profil
    Informations personnelles :
    Sexe : Homme
    Âge : 36
    Localisation : France, Rhône (Rhône Alpes)

    Informations professionnelles :
    Activité : Ingénieur développement logiciels
    Secteur : High Tech - Produits et services télécom et Internet

    Informations forums :
    Inscription : Avril 2008
    Messages : 2 211
    Points : 8 316
    Points
    8 316
    Billets dans le blog
    52
    Par défaut
    Pour moi, c'est du code relativement propre.

    Note : je ne suis pas développeur perl !

    Je n'ai que 2 points de détails à mentionné :

    1. Les variable magique.
    Que cela soit des String ou des nombres, il me semble préférable d'avoir une variable, une constante ou enum pour cela. Car, il y a toujours le problème du c'est la même string sauf que non.

    2. Consistance de l'indentation des if (en une ligne ou non)
    C'est du détail en particulier dans ce cas, où tout est proprement indenté.

    Cordialement,
    Patrick Kolodziejczyk.
    Si une réponse vous a été utile pensez à
    Si vous avez eu la réponse à votre question, marquez votre discussion
    Pensez aux FAQs et aux tutoriels et cours.

  6. #6
    Rédacteur/Modérateur

    Avatar de Lolo78
    Homme Profil pro
    Conseil - Consultant en systèmes d'information
    Inscrit en
    Mai 2012
    Messages
    3 612
    Détails du profil
    Informations personnelles :
    Sexe : Homme
    Localisation : France, Yvelines (Île de France)

    Informations professionnelles :
    Activité : Conseil - Consultant en systèmes d'information
    Secteur : High Tech - Opérateur de télécommunications

    Informations forums :
    Inscription : Mai 2012
    Messages : 3 612
    Points : 12 469
    Points
    12 469
    Billets dans le blog
    1
    Par défaut
    Bonjour,

    dans l'ensemble, le code est propre.

    Il y a pas mal de choses faites dans des modules non présentés, je ne peux pas commenter là-dessus.

    Quelques remarques sur les "bonnes pratiques" actuelles et d'autres points parfois assez secondaires, mais pas toujours.
    Code : Sélectionner tout - Visualiser dans une fenêtre à part
    1
    2
     
    #!/usr/bin/perl -w
    L'utilisation de l'option "-w" est périmée depuis une dizaine d'années, sauf pour les scripts unilignes. Il est bien préférable d'ajouter le pragma:
    vers le début de ton code. L'avantage principal, par rapport à l'option "-w" est que le pragma "use warnings" est lexical et n'est pas propagé automatiquement aux modules qui pourraient générer des warnings ou erreurs intempestifs. Il est également possible de le désactiver localement au besoin. Je sais que le tutoriel de Sylvain utilisait cette syntaxe, mais il l'a mis à jour assez récemment et a notamment remplacé les "-w" par des "use warnings;". Je t'engage à télécharger la dernière version de son tuto.
    Code : Sélectionner tout - Visualiser dans une fenêtre à part
    1
    2
     
    our ( $script_absolute_name, $script_relative_name, $script_dir, $script_name, $script_extension, $script_user ) = utils_script($0);
    Je ne vois aucune raison d'utiliser "our" pour ces variables, il est bien préférable d'utiliser des variables lexicales (déclarées avec "my"). Cette remarque s'applique aux autres cas où tu utilises "our", qui ne sont, sauf erreur, pas justifiés. (Mais je n'ai pas vu ce que font tes modules. S'ils utilisent ou modifient ces variables, ce n'est peut-être pas une bonne idée, il est généralement préférable de passer les variables et les valeurs de retour, mais ce n'est pas nécessairement une règle absolue.)
    Code : Sélectionner tout - Visualiser dans une fenêtre à part
    1
    2
    3
     
    my $envbatch = Shell::Source->new(shell => "ksh", file => "$ENV{'PROD_TECH_ENV'}/envbatch");
    $envbatch->inherit;
    Je doute que ce soit très portable sous Windows.
    Code : Sélectionner tout - Visualiser dans une fenêtre à part
    1
    2
     
    # Initialisation et création fichier log, puis redirection STDOUT et STDERR
    Tu ne rediriges que STDERR, pas STDOUT.
    Par ailleurs, pourquoi rediriger STDERR? Ne peux-tu pas t'arranger pour écrire directement là où tu as besoin? Je pose la question parce que l'utilisation de select et de "*STDERR = *$r_log;" est assez "magique" et peut-être à éviter quand on peut.
    Est-ce que ça compile? Est-ce que ça sert à quelque chose?
    Code : Sélectionner tout - Visualiser dans une fenêtre à part
    1
    2
    3
    4
    5
     
    sub process {
        my $dummy = 0;
        return;
    }
    A quoi sert cette fonction?

    Code : Sélectionner tout - Visualiser dans une fenêtre à part
    1
    2
     
    if ( $ARGV[0] eq "" ) { $reprise = h_const->{dft_reprise} } else { $reprise = $ARGV[0] };
    Perso, je réécrirais:
    Code : Sélectionner tout - Visualiser dans une fenêtre à part
    1
    2
    3
    4
    5
    6
     
    if ( $ARGV[0] eq "" ) { 
        $reprise = h_const->{dft_reprise};
    } else { 
        $reprise = $ARGV[0];
    }
    Mais c'est peut-être une affaire de goût personnel.

    Code : Sélectionner tout - Visualiser dans une fenêtre à part
    1
    2
    3
    4
    5
     
    use feature qw(switch say);
    ...
    when ('debut') {
    ...
    L'utilisation du switch a été "rétogradée" au rang de "fonctionnalité expérimentale" suscitant un warnings à partir, je crois, de la version 5.18 et susceptible de disparaître dans une prochaine version de Perl. A éviter donc pour du code supposé être pérenne.

    Il vaut mieux écrire des if successifs:
    Code : Sélectionner tout - Visualiser dans une fenêtre à part
    1
    2
    3
    4
    5
    6
    7
    8
    9
    10
     
    if ($reprise eq 'debut') {
        # ....
    } elsif ($reprise eq 'traitement {
        # ....
    } # ...
    } else {
        say "KO - Etiquette $reprise invalide";
        # ...
    }
    Si tu trouves la syntaxe un peu lourdingue, elle peut éventuellement être simplifiée comme suit:
    Code : Sélectionner tout - Visualiser dans une fenêtre à part
    1
    2
    3
    4
    5
    6
    7
    8
     
    for ($reprise) { # reprise est maintenant dans $_
        if (/debut/) {
            # ...
        } elsif (/traitement/) {
            # ...
        # ....
    }
    L'autre solution, un peu plus "avancée", est d'utiliser une table de distribution, c'est à dire un hachage contenant des références vers le code à exécuter. Un truc du genre de ce qu'il y a ci-dessous, que je ne peux pas tester (donc il y a peut-être des erreurs) car je n'ai qu'une partie de ton code et n'ai pas les données, mais ça donnera l'idée de base.

    Initialiser un hachage comme suit:
    Code : Sélectionner tout - Visualiser dans une fenêtre à part
    1
    2
    3
    4
    5
    6
    7
    8
    9
    10
    11
    12
    13
     
    my %r_balises_ok = 
        ("debut" =>
            sub { 
                oxa_msg_info( $r_log, 1, "===> Début de traitement" );
                next_step("traitement");
            },
         "traitement" =>
             sub { #....
            },
        "fin" => 
            # ...
        );
    Ensuite, dans la partie principale du traitement, un truc du genre:
    Code : Sélectionner tout - Visualiser dans une fenêtre à part
    1
    2
    3
    4
    5
    6
    7
    8
    9
    10
     
    until ( $reprise eq "fin" ) {
        oxa_msg_info( $r_log, 1,"LABEL : $reprise" );
        if (exists $r_balises_ok{$reprise) {
            $r_balises_ok{$reprise}->();
        } else {
            say "KO - Etiquette $reprise invalide";
           # ...
        }
    }
    Il est peu probable que le code ci-dessus fonctionne sans des ajustements, en particulier en ce qui concerne les variables locales et les passages de paramètres, mais ça devrait donner l'idée.

    Tu trouveras plus d'information sur les tables de distribution et sur la fonctionnalité switch dans ce tutoriel. Mais rien ne t'oblige à te lancer dans les tables de distribution si l'idée te paraît obscure ou si tu as du mal à maîtriser certaines choses, les deux autres solutions fournies sont bien plus simples.

    Voilà, j'espère que ça t'aide, mais n'hésite pas à demander si certains points ne sont pas clairs.

  7. #7
    Membre habitué
    Homme Profil pro
    Ingénieur intégration
    Inscrit en
    Mars 2015
    Messages
    138
    Détails du profil
    Informations personnelles :
    Sexe : Homme
    Localisation : France, Morbihan (Bretagne)

    Informations professionnelles :
    Activité : Ingénieur intégration
    Secteur : Service public

    Informations forums :
    Inscription : Mars 2015
    Messages : 138
    Points : 138
    Points
    138
    Par défaut Revue de code - modules
    Merci pour ces déjà précieux commentaires.
    Afin d'avoir une vue globale de ce modèle, je colle le code des 2 modules :

    Code : Sélectionner tout - Visualiser dans une fenêtre à part
    1
    2
    3
    4
    5
    6
    7
    8
    9
    10
    11
    12
    13
    14
    15
    16
    17
    18
    19
    20
    21
    22
    23
    24
    25
    26
    27
    28
    29
    30
    31
    32
    33
    34
    35
    36
    37
    38
    39
    40
    41
    42
    43
    44
    45
    46
    47
    48
    49
    50
    51
    52
    53
    54
    55
    56
    57
    58
    59
    60
    61
    62
    63
    64
    65
    66
    67
    68
    69
    70
    71
    72
    73
    74
    75
    76
    77
    78
    79
    80
    81
    82
    83
    84
    85
    86
    87
    88
    89
    90
    91
    92
    93
    94
    95
    96
    97
    98
    99
    100
    101
    102
    103
    104
    105
    106
    107
    108
     
    package enim_utils_v4;
    use strict;
    use Cwd 'abs_path';
    use File::Basename;
    use POSIX;
    use Net::Domain qw(hostname hostfqdn hostdomain);
     
    use Exporter;
    our @ISA = qw( Exporter );
    our @EXPORT = qw( &utils_script
                      &utils_log
                      &utils_nb_args
                      &utils_ctrl_balise
                      &utils_timestamp
                      &utils_month_2_num
                      &utils_split_hms
                      &utils_hostname
                      &utils_chk_dir
                      &utils_chk_file);
     
     
    sub utils_script {
        my ( $file ) = @_;
     
        my $script_absolute_name = abs_path ( $file );
        my $script_relative_name = basename ( $file );
        my ( $script_name, $script_dir, $script_extension ) = fileparse( $file, qr/\.[^.]*/);
        my $script_user = $ENV{'USER'};
     
        return ( $script_absolute_name, $script_relative_name, $script_dir, $script_name, $script_extension, $script_user);
    }
     
    sub utils_log {
        my ( $script, $pid ) = @_;
     
        my $dir_log      = "$ENV{'REPLOG'}/tech";
        my $date         = POSIX::strftime( "%Y%m%d_%H%M%S", localtime() );
        my $log          = "${dir_log}/${script}_${pid}_${date}.log";
        my $basename_log = "${script}_${pid}_${date}.log";
     
        open ( my $filew, ">", $log) || die "FATAL: cannot open \"$log\" for writing: $!\n";
        close( $filew) || die "Unable to close file : $log $!";
     
        return ( $basename_log, $log);
    }
     
     
    sub utils_nb_args {
        return( $#ARGV + 1 );
    }
     
     
    sub utils_ctrl_balise {
        my ( $balise, $r_balises_ok ) = @_;
     
        return ( grep( /^$balise$/, @$r_balises_ok) );
    }
     
     
    sub utils_timestamp {
        # Pour mémo, sur les deux lignes suivante, solution sans le module POSIX
        # my ($sec,$min,$hour,$mday,$mon,$year,$wday,$yday,$isdst) = localtime();
        # my $now = sprintf("%02d/%02d/%04d %02d:%02d:%02d", $mday, $mon+1, $year+1900, $hour, $min, $sec);
        return ( POSIX::strftime( "%d/%m/%Y %H:%M:%S", localtime()) );
    }
     
     
    sub utils_month_2_num {
        my ($m) = @_;
     
        my %h_mon_2_num = qw(jan 01  feb 02  mar 03  apr 04  may 05  jun 06  jul 07  aug 08  sep 09  oct 10  nov 11  dec 12 );
     
        return( $h_mon_2_num{ lc substr($m, 0, 3) } );
    }
     
     
    sub utils_split_hms {
        my ($h) = @_;
     
        my @t_h = split( /:/,$h );
     
        return($t_h[0], $t_h[1], $t_h[2]);
    }
     
    sub utils_hostname {
     
        my $fqdn_host = hostfqdn();
        my $host      = hostname();
     
        return ( $fqdn_host, $host );
    }
     
     
    sub utils_chk_dir {
        my ( $dir ) = @_;
     
        return ( -d $dir );
    }
     
     
    sub utils_chk_file {
        my ( $file ) = @_;
     
        return ( -f $file );
    }
     
    1;
    Code : Sélectionner tout - Visualiser dans une fenêtre à part
    1
    2
    3
    4
    5
    6
    7
    8
    9
    10
    11
    12
    13
    14
    15
    16
    17
    18
    19
    20
    21
    22
    23
    24
    25
    26
    27
    28
    29
    30
    31
    32
    33
    34
    35
    36
    37
    38
    39
    40
    41
    42
    43
    44
    45
    46
    47
    48
    49
    50
    51
    52
    53
    54
    55
    56
    57
    58
    59
    60
    61
    62
    63
    64
    65
    66
    67
    68
    69
    70
    71
    72
    73
    74
    75
    76
    77
    78
    79
    80
    81
    82
    83
    84
    85
    86
    87
    88
    89
    90
    91
    92
    93
    94
    95
    96
    97
    98
    99
    100
    101
    102
    103
    104
    105
    106
    107
    108
    109
    110
    111
    112
    113
    114
    115
    116
    117
    118
    119
    120
    121
    122
    123
    124
    125
    126
    127
    128
    129
    130
    131
    132
    133
    134
    135
    136
    137
    138
    139
    140
    141
    142
    143
    144
    145
     
    package oxa_utils_v4;
    use strict;
    use feature 'say';
    use Encode;
    use enim_utils_v4;
     
    use Exporter;
    our @ISA = qw(Exporter);
    our @EXPORT = qw( &oxa_main
                      &oxa_debut
                      &oxa_app
                      &oxa_msg_info
                      &oxa_msg_warn
                      &oxa_msg_err
                      &oxa_fin );
     
     
    sub oxa_debut {
        my ( $r_log, $script ) = @_;
     
        my ( $script_absolute_name, $script_relative_name, $script_dir, $script_name, $script_extension, $script_user ) = utils_script($script);
        my ( $fqdn_host, $host ) = utils_hostname();
     
        say $r_log "OXDAD:" . utils_timestamp();
        say $r_log "OXJOB:$script_relative_name";
        say $r_log "OXPWD:$script_dir";
        say $r_log "OXUSR:$script_user";
        say $r_log "OXSER:$host";
        say $r_log "VALUES:\[@ARGV\]";
     
        return;
    }
     
     
    sub oxa_app {
        my ( $r_log, $domaine, $application ) = @_;
     
        say $r_log "OXAPP:$domaine";
        say $r_log "OXSAP:$application";
     
        return;
    }
     
     
    sub oxa_msg_info {
        my ( $r_log, $flag, $msg ) = @_;
     
        my $tms = "";
        $tms = utils_timestamp() . " - "  if $flag;
        say $r_log "OXRES: " . $tms . "${msg}";
     
        return;
    }
     
    sub oxa_msg_warn {
        my ( $r_log, $flag, $msg ) = @_;
     
        my $tms = "";
        $tms = utils_timestamp() . " - "  if $flag;
        say $r_log "OXINC: " . $tms . "${msg}";
     
        return;
    }
     
     
    sub oxa_msg_err {
        my ( $r_log, $flag, $msg, $rc) = @_;
     
        my $tms = "";
        $tms = utils_timestamp() . " - "  if $flag;
        say $r_log "OXERR: " . $tms . "${msg}";
     
        oxa_fin( $r_log, $rc ) if $rc;
     
        return;
    }
     
    sub oxa_fin {
        my ( $r_log, $rc ) = @_;
     
        my $script_relative_name = "$main::script_relative_name";
        my $log                  = "$main::log_file";
        my $basename_log         = "$main::basename_log";
        my $dir_log_oxalys       = "$main::dir_log_oxalys";
     
        say $r_log "OXERR: Une erreur est survenue durant l'execution de $script_relative_name : RC=$rc" if $rc;
        say $r_log "OXDAF:" . utils_timestamp();
        say $r_log "OXSTA:$rc";
     
        # restauration des E/S standard
        select STDOUT;*STDERR=*STDOUT;
        close $r_log;
     
        # Récupération du nombre de lignes du fichier de log
        my $nb_lignes;
        open ( my $file_r, "<", $log) || die "FATAL: cannot open \"$log\" for reading: $!\n";
     
        while ( <$file_r> ) {$nb_lignes++};
        close $file_r;
     
        # Récupération de la taille du fichier de log
        #   si > 10 M , init du flag truncate pour conditionner la récup des 400 lignes du début et des 200 de la fin
        my $taille_log = -s $log;
        my ( $flag_truncate, $head_up, $head_down, $tail_up, $tail_down) = (0, 1, 400, 0, 0);
     
        if ( $taille_log >= 10485760 ) {
            $flag_truncate = 1;
            $tail_up       = $nb_lignes - 200 + 1;
            $tail_down     = $nb_lignes;
        }
     
        # Affichage écran de la log pour VTOM et sortie vers le répertoire scruté par
        #   Oxa en tenant compte du flag truncate
        open ( $file_r, "<:encoding(utf8)", $log) || die "FATAL: cannot open \"$log\" for reading: $!\n";
        my $logfile_oxa = "/ficdata/oxalys/log/${basename_log}";
        open ( my $file_w, ">:encoding(iso-8859-15)", $logfile_oxa) || die "FATAL: cannot open \"$logfile_oxa\" for writing: $!\n";
     
        while ( <$file_r> ) {
            my $flag_ecriture = 0;
     
            if ( $flag_truncate )  {
                if ( (($. >= $head_up) && ($. <= $head_down)) || (($. >= $tail_up) && ($. <= $tail_down)) ) {
                $flag_ecriture = 1;
              }
            } else {
                $flag_ecriture = 1;
            }
            if ( $flag_ecriture ) {
                print "$_";
                print $file_w "$_";
            }
        }
     
        close $file_r;
        close $file_w;
     
        exit $rc;
    }
     
    sub oxa_main {
        my $dummy = 0;
    }
     
    1;

  8. #8
    Membre habitué
    Homme Profil pro
    Ingénieur intégration
    Inscrit en
    Mars 2015
    Messages
    138
    Détails du profil
    Informations personnelles :
    Sexe : Homme
    Localisation : France, Morbihan (Bretagne)

    Informations professionnelles :
    Activité : Ingénieur intégration
    Secteur : Service public

    Informations forums :
    Inscription : Mars 2015
    Messages : 138
    Points : 138
    Points
    138
    Par défaut Revue de code - prise en compte au niveau du script
    - "use warnings;"
    je l'ai intégré dans le script et les modules

    - utilisation de "our"
    attente retours relecture du code des modules et recherche d'une autre solution

    - utilisation "Shell::Source"
    Effectivement, j'ai collé la version Linux du modèle.
    La version Windows est différente sur ce point

    - redirection E/S
    Code : Sélectionner tout - Visualiser dans une fenêtre à part
    1
    2
    my $r_log = \*$log;    # référence vers le fichier de log
      select $log;*STDERR = *$r_log;
    ces deux lignes redirigent bien STDIN et STDERR vers le fichier de log.
    J'avais trouvé cette solution en ligne, et elle fonctionne a priori bien

    Ne peux-tu pas t'arranger pour écrire directement là où tu as besoin
    Le but recherché était de s'affranchir des redirections dans tout le script
    et de rediriger de manière transparente toutes les sorties dans le fichier
    de log.
    Pour info, les scripts ksh utilisaient un système similaire de redirections
    que j'ai repris

    Code : Sélectionner tout - Visualiser dans une fenêtre à part
    1
    2
    3
    4
    5
    6
    # PROD - Sauvegarde des sorties std et stderr dans les fd 5 et 6
      exec 5>&1 6>&2 ; exec >> ${LOG} 2>&1
      [...]
      function rstlog {
        exec 1>&5 2>&6 ; exec 5>&- 6>&- ; pg $LOG
      }
    - oxa_main();
    ne sert à rien, à part à réserver une fonction aux éventuels pré-traitements
    nécessaires avant d'utiliser les autres fonctions oxa.
    Pour info, Oxalog est le serveur de logs maisons

    - sub process {
    ne sert à rien hormis à la démonstration

    - utilisation du switch
    les tables de distribution me sont inconnues.
    J'ai donc choisi dans un premier temps la simplicité :
    Code : Sélectionner tout - Visualiser dans une fenêtre à part
    1
    2
    3
    4
    5
    6
    7
    8
    9
    10
    # Boucle de traitement principale
      until ( $reprise eq 'fin' ) {
        oxa_msg_info( $r_log, 1,"LABEL : $reprise" );
     
        if ( $reprise eq 'debut') {
            oxa_msg_info( $r_log, 1, "===> Début de traitement" );
            next_step('traitement');
     
        } elsif ( $reprise eq 'traitement') {
        [...]

  9. #9
    Rédacteur/Modérateur

    Avatar de Lolo78
    Homme Profil pro
    Conseil - Consultant en systèmes d'information
    Inscrit en
    Mai 2012
    Messages
    3 612
    Détails du profil
    Informations personnelles :
    Sexe : Homme
    Localisation : France, Yvelines (Île de France)

    Informations professionnelles :
    Activité : Conseil - Consultant en systèmes d'information
    Secteur : High Tech - Opérateur de télécommunications

    Informations forums :
    Inscription : Mai 2012
    Messages : 3 612
    Points : 12 469
    Points
    12 469
    Billets dans le blog
    1
    Par défaut
    Citation Envoyé par ptonnerre Voir le message

    - utilisation de "our"
    attente retours relecture du code des modules et recherche d'une autre solution
    A priori, puisque la fonction du module renvoie les variables:
    Code : Sélectionner tout - Visualiser dans une fenêtre à part
    1
    2
     
    return ( $script_absolute_name, $script_relative_name, $script_dir, $script_name, $script_extension, $script_user);
    l'utilisation de my est bien meilleure:
    Code : Sélectionner tout - Visualiser dans une fenêtre à part
    1
    2
     
    my ( $script_absolute_name, $script_relative_name, $script_dir, $script_name, $script_extension, $script_user ) = utils_script($0);
    J'ai donc choisi dans un premier temps la simplicité :
    Tu as bien raison.

  10. #10
    Rédacteur/Modérateur

    Avatar de Lolo78
    Homme Profil pro
    Conseil - Consultant en systèmes d'information
    Inscrit en
    Mai 2012
    Messages
    3 612
    Détails du profil
    Informations personnelles :
    Sexe : Homme
    Localisation : France, Yvelines (Île de France)

    Informations professionnelles :
    Activité : Conseil - Consultant en systèmes d'information
    Secteur : High Tech - Opérateur de télécommunications

    Informations forums :
    Inscription : Mai 2012
    Messages : 3 612
    Points : 12 469
    Points
    12 469
    Billets dans le blog
    1
    Par défaut
    Citation Envoyé par ptonnerre Voir le message
    - redirection E/S
    Code : Sélectionner tout - Visualiser dans une fenêtre à part
    1
    2
    my $r_log = \*$log;    # référence vers le fichier de log
      select $log;*STDERR = *$r_log;
    ces deux lignes redirigent bien STDIN et STDERR vers le fichier de log.
    J'avais trouvé cette solution en ligne, et elle fonctionne a priori bien


    Le but recherché était de s'affranchir des redirections dans tout le script
    et de rediriger de manière transparente toutes les sorties dans le fichier
    de log.
    Oui, je ne doute pas que ça marche.

    Et c'est bien utile par exemple pour rediriger les sorties d'un module que tu ne maîtrises pas (fait par un tiers, partagé avec d'autres, etc.).

    Ici, tu maîtrises a priori tes modules, il vaut mieux leur passer la filehandle du fichier log et les faire écrire directement dans le fichier log. C'est moins "magique", plus naturel et plus clair à mon avis.

  11. #11
    Rédacteur/Modérateur

    Avatar de Lolo78
    Homme Profil pro
    Conseil - Consultant en systèmes d'information
    Inscrit en
    Mai 2012
    Messages
    3 612
    Détails du profil
    Informations personnelles :
    Sexe : Homme
    Localisation : France, Yvelines (Île de France)

    Informations professionnelles :
    Activité : Conseil - Consultant en systèmes d'information
    Secteur : High Tech - Opérateur de télécommunications

    Informations forums :
    Inscription : Mai 2012
    Messages : 3 612
    Points : 12 469
    Points
    12 469
    Billets dans le blog
    1
    Par défaut
    Sur ton premier module.

    Un nom de module commence en principe par une lettre capitale (comme Cwd ou File), sauf pour les modules dits pragmatiques (comme strict ou warnings). Ce n'est qu'une convention usuelle, mais ce n'est pas plus mal de s'y conformer.

    Code : Sélectionner tout - Visualiser dans une fenêtre à part
    1
    2
    3
    4
    5
     
    our @EXPORT = qw( &utils_script
                      &utils_log
                      # ...
                      &utils_chk_file);
    Il est inutile de précéder les noms de fonctions du et commercial (&):

    Code : Sélectionner tout - Visualiser dans une fenêtre à part
    1
    2
    3
    4
    5
     
    our @EXPORT = qw( utils_script
                      utils_log
                      # ...
                      utils_chk_file);
    Par ailleurs, la décision de conception t'appartient, mais il est souvent recommandé de ne pas exporter ainsi systématiquement toute une tripotée de fonctions, mais plutôt de les mettres dans un tableau @EXPORT_OK et, ensuite, dans le programme utilisant le module, de nommer les fonctions que tu utilises vraiment:

    Code : Sélectionner tout - Visualiser dans une fenêtre à part
    1
    2
    3
    4
    5
     
    our @EXPORT_OK = qw( utils_script
                      utils_log
                      # ...
                      utils_chk_file);
    Et, dans le programme appelant:
    Code : Sélectionner tout - Visualiser dans une fenêtre à part
    1
    2
     
    use enim_utils_v4 qw (utils_script ...);
    Cela peut éviter d'éventuelles collisions de noms entre deux modules et ça documente dans ton programme principal d'où vient la fonction que tu utilises.
    Code : Sélectionner tout - Visualiser dans une fenêtre à part
    1
    2
    3
     
        open ( my $filew, ">", $log) || die "FATAL: cannot open \"$log\" for writing: $!\n";
        close( $filew) || die "Unable to close file : $log $!";
    A quoi ça sert d'ouvrir un fichier et de fermer immédiatement? Juste à vérifier que tu arrives bien à le créer? Tu peux aussi bien le faire dans le programme principal...

    Code : Sélectionner tout - Visualiser dans une fenêtre à part
    1
    2
    3
    4
     
    sub utils_nb_args {
        return( $#ARGV + 1 );
    }
    Ecrire une fonction juste pour faire cela me paraît un peu chercher à faire compliqué quand on peut faire simple. Par ailleurs, "scalar @ARGV" te renvoie directement le nombre d'arguments. Et le mot clef scalar n'est même pas nécessaire si tu es en contexte scalaire. Par exemple:
    Code : Sélectionner tout - Visualiser dans une fenêtre à part
    1
    2
     
    die "Usage: Le nombre d'arguments attendu est 2\n" unless @ARGV == 2;
    fonctionnera parce que l'opérateur de comparaison numérique "==" impose un contexte scalaire à @ARGV qui renverra dans ce cas le nombre de ses éléments.
    Code : Sélectionner tout - Visualiser dans une fenêtre à part
    1
    2
    3
    4
    5
    6
    7
     
    sub utils_timestamp {
        # Pour mémo, sur les deux lignes suivante, solution sans le module POSIX
        # my ($sec,$min,$hour,$mday,$mon,$year,$wday,$yday,$isdst) = localtime();
        # my $now = sprintf("%02d/%02d/%04d %02d:%02d:%02d", $mday, $mon+1, $year+1900, $hour, $min, $sec);
        return ( POSIX::strftime( "%d/%m/%Y %H:%M:%S", localtime()) );
    }
    Pareil, tu fais comme tu veux, mais mettre dans un module une fonction qui n'a en fait qu'une seule ligne de code est un peu de l'overkill. A la rigueur, je le mettrais dans une fonction séparée de mon programme principal si c'est utilisé plusieurs fois, mais dans un module, ça fait beaucoup et ça finit par compliquer le débogage.
    Code : Sélectionner tout - Visualiser dans une fenêtre à part
    1
    2
    3
    4
    5
    6
    7
    8
     
    sub utils_split_hms {
        my ($h) = @_;
     
        my @t_h = split( /:/,$h );
     
        return($t_h[0], $t_h[1], $t_h[2]);
    }
    Juste pour info, la ligne de retour peut aussi s'écrire avec une tranche de tableau:
    Code : Sélectionner tout - Visualiser dans une fenêtre à part
    1
    2
    3
    4
    5
    6
     
    sub utils_split_hms {
        my $h = shift;
         my @t_h = split /:/,  $h;
         return $t_h[0..2];
    }
    Remarque aussi la suppression de quelques parenthèses inutiles.

    Ou tu peux même écrire:
    Code : Sélectionner tout - Visualiser dans une fenêtre à part
    1
    2
    3
    4
    5
     
    sub utils_split_hms {
        my $h = shift;
        return (split  /:/, $h )[0..2];
    }
    Ou même:

    Code : Sélectionner tout - Visualiser dans une fenêtre à part
    1
    2
    3
    4
     
    sub utils_split_hms {
        return (split  /:/, shift)[0..2];
    }
    Mais à ce point de simplification, faut-il encore une fonction dans un module pour une seule ligne de code?

    De même,
    Code : Sélectionner tout - Visualiser dans une fenêtre à part
    1
    2
    3
    4
    5
    6
    7
    8
     
    sub utils_hostname {
     
        my $fqdn_host = hostfqdn();
        my $host      = hostname();
     
        return ( $fqdn_host, $host );
    }
    peut s'écrire:
    Code : Sélectionner tout - Visualiser dans une fenêtre à part
    1
    2
    3
    4
     
    sub utils_hostname {
        return ( hostfqdn(), hostname() );
    }
    Tes deux dernières fonctions peuvent être simplifiées de façon similaire. Par exemple, la dernière:
    Code : Sélectionner tout - Visualiser dans une fenêtre à part
    1
    2
    3
    4
     
    sub utils_chk_file { 
         -f shift 
    }
    Toutes les simplifications énoncées ne sont absolument pas nécessaires, tes fonctions étaient très bien, je voulais juste te montrer que tu pouvais écrire du code plus concis et restant, à mon avis, très lisible.

  12. #12
    Membre habitué
    Homme Profil pro
    Ingénieur intégration
    Inscrit en
    Mars 2015
    Messages
    138
    Détails du profil
    Informations personnelles :
    Sexe : Homme
    Localisation : France, Morbihan (Bretagne)

    Informations professionnelles :
    Activité : Ingénieur intégration
    Secteur : Service public

    Informations forums :
    Inscription : Mars 2015
    Messages : 138
    Points : 138
    Points
    138
    Par défaut Merci
    MERCI à vous pour vos contributions qui vont m'aider à avancer.
    Je vais intégrer et mettre à jour mes modèles et reviens dans le fil si besoin pour des points de détail.

  13. #13
    Membre habitué
    Homme Profil pro
    Ingénieur intégration
    Inscrit en
    Mars 2015
    Messages
    138
    Détails du profil
    Informations personnelles :
    Sexe : Homme
    Localisation : France, Morbihan (Bretagne)

    Informations professionnelles :
    Activité : Ingénieur intégration
    Secteur : Service public

    Informations forums :
    Inscription : Mars 2015
    Messages : 138
    Points : 138
    Points
    138
    Par défaut Revue de code
    Le module Oxa regroupe toutes les fonctions à utiliser pour l'écriture des
    logs destinées à notre serveur de log L'autre module est une boite à outils
    des fonctions.

    En ce qui concerne l'utilisation de "our", les variables retournées par la
    fonction utils_script seront utilisées par la fonction oxa_fin lors d'une
    sortie normale du script ou après une sortie en erreur, via la fonction
    oxa_msg_error.
    Or, cette dernière fonction peut être appelée de nombreuses fois dans un
    script et je trouve que cela alourdirait le code de passer ces arguments à
    chaque appel d'oxa_msg_error.
    Je ne vois pas dans l'immédiat de solution : une idée ?

    Pour les redirections des E/S, cela est assez magique et pratique en effet.
    Toutes les sorties sont redirigées automatiquement vers le fichier de log sans
    alourdir le code.
    Nos scripts sont lancés via des automates de production
    (VTOM, $UNIVERSE...) qui interceptent la sortie standard : dans la fonction
    oxa_fin, il nous suffit de lire et d'afficher cet unique fichier de log pour
    avoir son contenu disponible dans la log de l'ordonnanceur, nous évitant de
    nous connecter aux serveurs pour voir la log des traitements.

    J'ai modifié mes exports de symboles en mode individuel et effectivement, cela
    documente bien le code.

    Quand j'ouvre et ferme tout de suite le fichier de log, c'est juste pour le créer
    et avoir une 1ère trace dans notre serveur de logs.

    La fonction utils_timestamp est appelée de nombreuses fois par les fonctions
    oxa et nous a déjà servie dans d'autres scripts.

    Tous les autres exemples de simplification ont été repris et mon code est, il
    est vrai, plus clair et plus concis.
    Je vais essayer de prendre du temps pour relire les tutos ;-)

    MERCI et to be continued ..

  14. #14
    Membre habitué
    Homme Profil pro
    Ingénieur intégration
    Inscrit en
    Mars 2015
    Messages
    138
    Détails du profil
    Informations personnelles :
    Sexe : Homme
    Localisation : France, Morbihan (Bretagne)

    Informations professionnelles :
    Activité : Ingénieur intégration
    Secteur : Service public

    Informations forums :
    Inscription : Mars 2015
    Messages : 138
    Points : 138
    Points
    138
    Par défaut
    Avant de clore ce fil, avez-vous des remarques sur les règles de nommage ?

  15. #15
    Rédacteur/Modérateur

    Avatar de Lolo78
    Homme Profil pro
    Conseil - Consultant en systèmes d'information
    Inscrit en
    Mai 2012
    Messages
    3 612
    Détails du profil
    Informations personnelles :
    Sexe : Homme
    Localisation : France, Yvelines (Île de France)

    Informations professionnelles :
    Activité : Conseil - Consultant en systèmes d'information
    Secteur : High Tech - Opérateur de télécommunications

    Informations forums :
    Inscription : Mai 2012
    Messages : 3 612
    Points : 12 469
    Points
    12 469
    Billets dans le blog
    1
    Par défaut
    J'ai déjà signalé la majuscule pour l'initiale des modules (hors pragmas).

    Je n'ai rien vu d'autre, les noms sont plutôt parlants et informatifs.

  16. #16
    Membre averti
    Avatar de magicshark
    Homme Profil pro
    Dans une SS2I donc pas que JAVA
    Inscrit en
    Octobre 2011
    Messages
    133
    Détails du profil
    Informations personnelles :
    Sexe : Homme
    Localisation : France

    Informations professionnelles :
    Activité : Dans une SS2I donc pas que JAVA
    Secteur : High Tech - Éditeur de logiciels

    Informations forums :
    Inscription : Octobre 2011
    Messages : 133
    Points : 320
    Points
    320
    Par défaut
    Il existe aussi ce site (qui utilise perl critic) pour vérifier ton code avec différent niveau de vérification :
    http://perlcritic.com/
    Pourquoi faire simple quand on peut faire compliqué.

+ Répondre à la discussion
Cette discussion est résolue.

Discussions similaires

  1. Outil pour la revue de code
    Par FABFAB125 dans le forum Outils
    Réponses: 7
    Dernier message: 25/11/2007, 10h35
  2. Outils de revue de code
    Par grabriel dans le forum EDI, CMS, Outils, Scripts et API
    Réponses: 2
    Dernier message: 22/08/2007, 11h56
  3. Outils de revue de code
    Par YAMKI dans le forum Qualimétrie
    Réponses: 2
    Dernier message: 15/02/2006, 12h29
  4. [Conseil] revue de code
    Par allstar dans le forum Langage
    Réponses: 2
    Dernier message: 09/11/2005, 11h02
  5. [Revue de code] Quels outils pour de grosses applis?
    Par franckR dans le forum Choisir un environnement de développement
    Réponses: 1
    Dernier message: 21/03/2004, 10h03

Partager

Partager
  • Envoyer la discussion sur Viadeo
  • Envoyer la discussion sur Twitter
  • Envoyer la discussion sur Google
  • Envoyer la discussion sur Facebook
  • Envoyer la discussion sur Digg
  • Envoyer la discussion sur Delicious
  • Envoyer la discussion sur MySpace
  • Envoyer la discussion sur Yahoo