Christianhv Christianhv - 4 months ago 19
PowerShell Question

rename and copy Powershell - too complex script

I wrote a script to copy a file, but this file already exists in the target directory. So, first I need to rename the original file.

The script works, but a colleague did not want to use it because it is too complex. I would like to know how can I improve the code of this script. In some sense it seems to be overkilled for simple task. I would like to find an objective reason to that opinion. The code is like this:

###GLOBAL VARIABLES

$goodDir = "..."
$targetDir = "..."
$goodFile = "..."
$targetFile = "..."

$app = "..."
$logDir= "..."
$timestamp = Get-Date -Format "dd-MMM-yyyy HH:mm:ss.fff" | foreach {$_ -replace ":", "."}

$log_name = $app + "_pscp-" + $timestamp + ".log"
$log_file = $logDir + $log_name



########################
#FUNCTIONS
########################



Function WriteLog
{
param(

[Int]$ecode=0,
[string]$msg="Error Message"
)
$now = Get-Date -format "dd-MMM-yyyy HH:mm:ss.fff"
$messageToLog = $now + ":[exit code " + $ecode.ToString() + "] :`t" + $msg
Add-Content $log_file -value $messageToLog
}

function rename-File {
param(
# file to copy
[Parameter(Mandatory)]
[string]$fname,
[Parameter(Mandatory)]
[string]$from
)
$now = Get-Date -format "dd-MMM-yyyy"
$toNewName = $fname + "-Copy-" + $now
try {
Rename-Item -Path $from$fname $toNewName -ErrorAction Stop
}
catch {
WriteLog -ecode 4 -msg "The item $_.Exception.ItemName failed with the following message $_.Exception.Message"
Break
}
Finally {
WriteLog -msg "Rename FILE $fName `r`n`t`t`t`t`t`tFROM $from `r`n`t`t`t`t`t`tTO $toNewName"
}

}



function copy-File {
param(
# file to copy
[Parameter(Mandatory)]
[string]$fname,
[Parameter(Mandatory)]
[string]$from,
[Parameter(Mandatory)]
[string]$to )

try {
Copy-Item -Path $from$fname -Destination $to -ErrorAction Stop
}
catch {
WriteLog -ecode 4 -msg "The item $_.Exception.ItemName failed with the following message $_.Exception.Message"
Break}
Finally {
WriteLog -msg "Copy FILE $fName `r`n`t`t`t`t`t`tFROM $from `r`n`t`t`t`t`t`tTO $to"
}

}

### MAIN ###


rename-File -fname $targetFile -from $targetDir
copy-File -fname $goodFile -from $goodDir -to $targetDir$targetFile
`

Answer

Here are some tipps:

  • Try to avoid / minimize the amout of global variables.
  • Use approved verbs (see Get-Verb) for your functions (the first letter should be uppercase).
  • Use the Join-Path cmdlet to combine a path.
  • Use string interpolation

    "${app}_pscp-$timestamp.log"
    

    or the format operator

    '{0}_pscp-{1:dd-MMM-yyyy HH.mm.ss.fff}.log' -f $app, (Get-Date)
    

    instead of string concatenation.

Also, I would not create a Copy-File nor a Rename-File function. Just invoke Copy-Item and Rename-Item in the main…