Discussion :

[vince2005]

Bonjour,

Est ce que vous connaissez une façon d'écrire ce code plus propre :

Autrement dit, je souhaiterai aligner ma variable $Dump_Base_SQL sur l'ensemble de mon script ou faire des appels via des fonctions. Comment puis je faire ?

Merci

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
 
 
my @TableauLigne;
my $Finish_Task_Archiving = "XX=" ;
open(FIC, "<${TempSql}/${FicSqlTmp}") || die "Ouverture impossible du fichier ${FicSqlTmp}: $!";	
 
				while ($Ligne = <FIC> ) 
   				{  		
					if (grep(/$Finish_Task_Archiving/, $Ligne)) 
					{
						#Supprime les espaces et tabulations pour remplacer par ¤
						$Ligne =~ s/[\t\s]//g;
						@TableauLigne=split(/\XX=/,$Ligne);
						my $UneBase=$TableauLigne[1];
						print ">${UneBase}<\n";
 
 
my $Dump_Base_SQL = <<"SQL1";
use master;
BACKUP DATABASE ${UneBase} TO  DISK = N'${TempSql}/${Instance}_${UneBase}.dmp' 
WITH NOFORMAT, INIT,  NAME = N'dump-Complet ${UneBase}', 
SKIP, NOREWIND, NOUNLOAD,  STATS = 10 ;
SQL1
 
						print "$Dump_Base_SQL\n";
						my $CmdDump=system("$SqmCmd -S${Host}\\${Instance} -Q \"${Dump_Base_SQL}\" -o \"${TempSql}/${UneBase}_${FicSqlDump}\"") ;
					}
				}
close (FIC);

[ggnore]

Tu pourrais rajouter, pour commencer :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
use strict;
use warnings;

Ta façon d'écrire $Dump_Base_SQL ressemble à du Bash. Pourquoi ne pas utiliser ce que permet Perl ?

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
my $insert_deb =    "use master;\n".
"BACKUP DATABASE ${UneBase} TO  DISK = N'${TempSql}/${Instance}_${UneBase}.dmp'\n".
"WITH NOFORMAT, INIT,  NAME = N'dump-Complet ${UneBase}',\n". 
"SKIP, NOREWIND, NOUNLOAD,  STATS = 10 ;"

Le . permet de concaténer. N'hésite pas à mettre tes appels de fonctions sur plusieurs lignes pour une meilleure lisibilité.
Ton indentation n'est pas très cohérente.

[Lolo78]

++, commence par utiliser:

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
use strict;
use warnings;

et par déclarer toutes tes variables lexicalement (avec "my") dans le scope le plus étroit possible. Il ne semble, par exemple, pas nécessaire de déclarer @TableauLigne comme une variable globale au début du script, alors que cette variable n'est utilisée qu'à l'intérieur du bloc if lui même à l'intérieur du bloc while.

J'essaierai de revenir quand je serai sur un vrai ordi pour te proposer une version plus conforme aux bonnes pratiques communément admises par le communauté Perl.

[Lolo78]

Bonjour,

voici une première proposition avec des commentaires dans le code.

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
#!/usr/bin/perl
 
use strict;
use warnings;
 
# je ne sais pas d'où viennent toutes les variables ci-dessous, non déclarées et non initialisées
# j'ai ajouté leur déclaration pour tester la compilation mais ça ne fonctionnera pas pour
# autant tant qu'elles ne sont pas initialisées
my ($TempSql, $FicSqlTmp, $Instance, $SqmCmd, $Host, $FicSqlDump);
 
my $Finish_Task_Archiving = "XX=" ;
open my $FIC, "<", "$TempSql/$FicSqlTmp" or die "Ouverture impossible du fichier $FicSqlTmp: $!";
# Note: j'ai modernisé la syntaxe d'ouverture du fichier et utilisé un FH lexical ($FIC)
 
 
while (my $Ligne = <$FIC> ) 
{  		
	if (grep(/$Finish_Task_Archiving/, $Ligne)) 
	{
		$Ligne =~ s/\s//g; # \s est une classe de caractères qui reconnaît les espaces et les tabulations
		my $UneBase = (split /\XX=/, $Ligne)[1];
		print ">${UneBase}<\n";
 
my $Dump_Base_SQL = <<"SQL1";
use master;
BACKUP DATABASE $UneBase TO  DISK = N'$TempSql/${Instance}_$UneBase.dmp' 
WITH NOFORMAT, INIT,  NAME = N'dump-Complet $UneBase', 
SKIP, NOREWIND, NOUNLOAD,  STATS = 10 ;
SQL1
 
# note: d'où viennent les variables $TempSql et ${Instance}
 
		print "$Dump_Base_SQL\n";
		my $CmdDump = system("$SqmCmd -S$Host\\$Instance -Q \"$Dump_Base_SQL\" -o \"$TempSql/${UneBase}_$FicSqlDump\"") ;
	}
}
close ($FIC);

J'ajoute que ton "heredoc" au milieu de la boucle while rend le formatage peu lisible. Perso, je déclarerais un modèle pour cette commande avec un emplacement pour la base au début puis ferais la substitution au moment où la base est connue. Dans la seconde proposition ci-dessous, j'ai aussi remplacé le bloc if par un next quand la condition n'est pas satisfaite, une imbrication de bloc en moins rend le code plus lisible à mes yeux (mais ce n'est que mon avis personnel). En définitive, quelque chose comme cela:

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
#!/usr/bin/perl
use strict;
use warnings;
 
# je ne sais pas d'où viennent toutes les variables ci-dessous, non déclarées et non initialisées
# j'ai ajouté leur déclaration pour tester la compilation mais ça ne fonctionnera pas pour
# autant tant qu'elles ne sont pas initialisées
my ($TempSql, $FicSqlTmp, $Instance, $SqmCmd, $Host, $FicSqlDump);
 
my $modele_SQL = <<"SQL1";
use master;
BACKUP DATABASE LABASE TO  DISK = N'$TempSql/${Instance}_LABASE.dmp' 
WITH NOFORMAT, INIT,  NAME = N'dump-Complet LABASE', 
SKIP, NOREWIND, NOUNLOAD,  STATS = 10 ;
SQL1
 
open my $FIC, "<", "$TempSql/$FicSqlTmp" or die "Ouverture impossible du fichier $FicSqlTmp: $!";
 
while (my $Ligne = <$FIC> ) 
{  		
	next unless $Ligne =~ /XX=/; # une expression régulière aussi simple, autant la coder en dur, c'est plus clair et souvent plus rapide (elle n'est compilée qu'une fois)
	$Ligne =~ s/\s//g; # \s est une classe de caractères qui reconnaît les espaces et les tabulations
	my $UneBase = (split /\XX=/, $Ligne)[1];
	print ">${UneBase}<\n";
	my $Dump_Base_SQL = $modele_SQL;
	$Dump_Base_SQL =~ s/LABASE/$UneBase/g;
	print "$Dump_Base_SQL\n";
	my $CmdDump = system("$SqmCmd -S$Host\\$Instance -Q \"$Dump_Base_SQL\" -o \"$TempSql/${UneBase}_$FicSqlDump\"") ;
}
close ($FIC);

A mon avis, ça rend la logique du programme bien plus lisible. Et la boucle while est maintenant suffisamment courte pour qu'il ne paraisse pas utile de déplacer une partie du code dans une fonction.

Dernier point: Perl n'est pas du shell, il n'est pas utile d'utiliser des accolades autour du nom des variables, sauf s'il y a un risque d'ambiguïté: $UneBase fonctionne bien, inutile d'utiliser ${UneBase}, cela nuit à la lisibilité. (Je sais que ce n'est pas toujours indispensable non plus en shell, mais on le fait très souvent en shell pour éviter des problèmes qui ne se posent généralement pas en Perl.)