4
\$\begingroup\$

I have a script for zipping up old files. I know using Windows Compression isn't ideal, so I will make the script run using 7-Zip later on. For now though, I just want see how I can make my current script better. By better I mean, how could I make this code cleaner or neater? How could I make the script more effective? Here is the code:

Write-Output("Beginning script....")

#File path of file to be cleaned
$File_Path = "C:\Users\Administrator\Downloads\Testing\*"

#Location of ZIP file
$Send_To = "C:\Users\Administrator\Documents\ARCHIVE2"

#Location of old files before being zipped
$Old_Files = "C:\Users\Administrator\Documents\OLD_FILES"

#Time frame for files
$Days = "-65"
$now = Get-Date
$last_Write = $now.AddDays($Days)

#Filtering files according to time parameters
$Filter = Get-ChildItem -Path $File_Path | Where-Object { $_.LastWriteTime -lt $last_Write }

#Moving old files to destination folder
if (!$Filter){
    Write-Host "Variable is null"
}
else{
    Move-Item $Filter -Destination $Old_Files
    Write-Output("Moving files....")
}



#Compressing destination folder if there is folders in $Old_Files folder
if (!(Test-Path $Old_Files)) {
    Write-Output("No old files found")
}
else{
    Compress-Archive -Path $Old_Files -DestinationPath $Send_To -Update
    Write-Output("Old files zipped!")
    Remove-Item -Path $Old_Files -Force
}

Write-Output("Script is finished")
\$\endgroup\$
  • 1
    \$\begingroup\$ Omit the #Moving old files to destination folder part. Compress-Archive accepts pipeline input (ByPropertyName) instead of -Path/-LiteralPath. Use $Filter | Compress-Archive -DestinationPath $Send_To -Update -CompressionLevel Optimal. For planned 7-Zip, use its @{listfile} facility; create an auxiliary list file from $Filter.FullName using UTF8 encoding (7-Zip's default). \$\endgroup\$ – JosefZ Mar 21 at 15:22
2
\$\begingroup\$

I spent several weeks writing and tuning a script that moves old files to a timestamped Zip file. Here are a few guidelines I learned that may help:

1) Move variables into script parameters so they can be changed at runtime without editing the file:

Param($File_Path = "C:\Users\Administrator\Downloads\Testing\*", #File path of file to be cleaned    
  $Send_To = "C:\Users\Administrator\Documents\ARCHIVE2", #Location of ZIP file
  $Old_Files = "C:\Users\Administrator\Documents\OLD_FILES", #Location of old files before being zipped
  $Days = "-65"
)

2) Filter usually means 'criteria for filtering a list'. Calling the list of files $Filter is misleading. $FilesToZip might be a better variable name.

3) There are lots of reasons a file can't be moved or zipped (in use, doesn't exist, no read/write permission, etc.) You should have a try-catch block around Compress-Archive to account for this, and fail or proceed gracefully. Using ZipFileExtensions, an error created an unusable file until it was finalized. Compress-Archive may be more robust, but there are still chances for failure (try pulling your network cable while creating a large archive of files on your LAN).

4) There's no need for an intermediate folder, and it creates more opportunities for failure. You can just compress the file into the zip and delete on success.

5) The message 'No old files found' is misleading. Reading it, I would think that there were no files found matching the age criteria, when it really means the intermediate folder doesn't exist.

6) Don't you need a .Zip file name for the -DestinationPath?

7) Write-Output is usually used for return values rather than messages. Write-verbose, or Write-Host are more appropriate for status messages.

\$\endgroup\$
  • \$\begingroup\$ Thanks for the input \$\endgroup\$ – techguy1029 May 6 at 21:13

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.