Discussion :

[Radsgord]

Bonjour à tous,

J'ai réalisé un script mais je souhaiterais avoir un regard de spécialiste et objectif afin de m'aider à écrire du code correct.

Puis-je donc partager mon code afin que vous puissiez y apporter les corrections et m'indiquer les bonnes pratiques à adopter.

Merci de commenter la syntaxe lors des modifications pour un apprentissage plus fluide de ma part.

Merci à tous.

J'attends vos réponse avant de partager le script Powershell.

[Alastor2262]

Moi je suis tenté

[Radsgord]

Moi je suis tenté

Bonjour Alastor2262,

Merci beaucoup d'accepter de regarder mon script.

Je joint le script.

merci par avance.

[Alastor2262]

Bonjour,

Je vais regarder d'ici ce week-end, j'ai toutefois deux remarques d'ordre général.

Pourquoi stocker les ordinateur dans un fichier puis faire sans cesse un "get-content" ? les garder dans une variable serait plus efficace.
Ensuite, pourquoi ne pas faire du "full" powershell ? get-wmi et "net view" me semble contre productif, non ?

[Radsgord]

Bonsoir,

Merci à toi.

Concernant tes remarques :


J'ai stocké les ordinateurs dans un fichier pour me permettre dans un premier temps de bien visualiser les différentes étapes et être certain d'obtenir ce que je souhaitais.

Net View : Car je connaissais déjà cette commande sous cmd et j'ai souhaitais m'appuyer dessus dans un premier temps.
A remplacer peut-être par quelque chose comme ceci :

Code :

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

1
2
 $cim = New-CimSession -ComputerName $Computers
Get-SmbShare -CimSession $cim | Select Name

Get-Wmi à remplacer par Get-CimInstance je pense.

Comme tu as pu aisément le constater, ce code et un assemblage de petit tests que je fais.
Au fur et à mesure je me dis que je dois pouvoir faire plus et j'avance ainsi ( tel un nouveau né à quatre pattes ), je ne désespére pas de progresser et d'arriver à me tenir debout

Je suis conscient de l’extrême médiocrité de la syntaxe de ce script qui et probablement lui-même le reflet d'une mauvaise façon de penser... le code.
Je viens de découvrir Powershell depuis peu et j'adore cela, donc je m'obstine ,

Pas de réels besoins perso où professionnel juste l'envie d'apprendre et de progresser.

Voila en espérant avoir répondu à tes remarques.

Encore une fois merci beaucoup à toi.

[Alastor2262]

J'avoue ne pas prendre de pincette, ni mettre des fleurs, sinon le sujet du powershell serait perdu dans le "bruit" , mais il n'y a pas de jugement de valeur, seulement un avis, des idées.

Je te rassure, quand je reprends mes scripts quelques semaines plus tard, je me demande souvent : mais pourquoi j'ai fait ça comme ça ! il y a mieux ! Il y a aussi l'apprentissage, qui fait que l'on ne fait plus les choses de la même manière.

Pour revenir à ton script, j'ai un problème... je ne sais pas ce qu'il fait. Il y a un cartouche d'information au début, mais pas, en deux phrase, ce que le script fait...

Mon incapacité à comprendre ce qu'il fait vient probablement du fait que tu fais une interface graphique et que pour ma part, je n'en ai jamais fait powershell. Toutefois, si je devait en faire, je séparerais du mieux possible le code et l'interface, justement pour la lisibilité, la compréhension et la maintenance.
Typiquement, si c'est possible, le "Add_Click" ne ferais qu'extraire les données, pour les passer en paramètre à une fonction, le retour de la fonction allant renseigner l'interface toujours dans le Add_click...
Je ne sais pas si c'est strictement des bonnes pratiques powershell, mais c'est ainsi que je ferais.

Ensuite, à quoi te serve les commandes suivantes au début du script ?

Code :

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

1
2
3
4
5
6
7
8
9
10
11
12
$WhatIfPreference = $false

## Type d'execution
Set-ExecutionPolicy Unrestricted -Scope CurrentUser -Force

# Nettoyage Terminal
Clear-Host

## Création des variables d'environnement 
[Environment]::SetEnvironmentVariable("Documents", "$env:USERPROFILE\Documents", "User")
[Environment]::SetEnvironmentVariable("Bureau", "$env:USERPROFILE\Desktop", "User")

[Radsgord]

Bonsoir Alastor2262,

Désolé de ce retour très tardif, je ne reprends le clavier qu'aujourd'hui.

Suivant tes remarques, sous le cartouches je vais rajouter
Objectif du Script :
1) ...
2) ...
Exemple :
1) ...
2) ...

Concernant l'interface graphique, c'est en partie là que réside le soucis car c'est mon premier script avec Interface Graphique.

Code :

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

$WhatIfPreference = $false

Simule et commente la commande ne l'excute pas réellement.

Code :

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

1
2
# Nettoyage Terminal
Clear-Host

Je nettoie le terminal de Vscode avant de lancer le script des anciens messages.(Write-Warning, Write-Error..)

Code :

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

1
2
3
## Création des variables d'environnement 
[Environment]::SetEnvironmentVariable("Documents", "$env:USERPROFILE\Documents", "User")
[Environment]::SetEnvironmentVariable("Bureau", "$env:USERPROFILE\Desktop", "User")

Une erreur que j'ai laissé en reprenant le cartouche d'un ancien script.

Je vais de ce pas retourner dessus et te renvoyer les modifications apportées si cela ne te dérange toujours pas.

Merci.

[Alastor2262]

Pas de problème, tu peux re-envoyer.