Zip the contents of subfolders, conditionally

I've written a script in Powershell for zipping files into new folders.

This script performs the following steps:

• Check whether each subfolder contains non-zip files which are older than 31 days

• If so, create a new folder within, labelled with the date

• Move the old files to the newly-created folder

• Create a zipped copy of said folder

• Delete the unzipped source folder

If I could get some feedback, that'd be great!

$initpath = get-location #determine initial location, in order to return later
foreach($path in get-childitem -recurse) { #get all subfolders
if ($path.Attributes -eq "Directory") { 
    set-location $path.FullName 
    $date = "$((Get-Date).ToString('yyyy-MM-dd_hh-mm'))"
    $newpath = [io.path]::combine("$path$date","")

    #get all items older than 31 days, exclude zip files and folders
    $items = Get-ChildItem -Exclude *.zip | ? {$_.LastWriteTime -lt (Get-date).AddDays(-31) -and -not $_.psIsContainer}

    #verify that there are such items in this directory
    if ( $(Try { Test-Path $items } Catch { $false }) ) {
        $newfld = New-Item -ItemType Directory -name $newpath

        Move-Item $items -destination $newpath #move items to newly-created folder

        #the following block zips the folder
        $src = $newfld.FullName
        $dest = "$src.zip"
        [Reflection.Assembly]::LoadWithPartialName( "System.IO.Compression.FileSystem" )
        [System.IO.Compression.ZipFile]::CreateFromDirectory($src, $dest)

        Remove-Item $src -force -recurse #remove the unzipped source folder
    }
    Else {
    Continue #continue to the next folder if there are no relevant files present
    }

    }
}
set-location $initpath #return to the initial location

Path Location

I see that you're trying to save the current location and then restore it later.

There are actually built-in cmdlets for this, Push-Location and Pop-Location. They work the same way as pushd and popd if you're familiar with those from the regular Windows command prompt for from unix-like systems (and in fact those terms are aliases in PowerShell and still work).

So you can actually just do this:

Push-Location  # saves the current location
# do some stuff
Pop-Location   # restores the original location

You can also give a location to change to, so for example instead of doing Set-Location later, you can do something like this:

$dirToChangeTo = 'C:\Windows'
Push-Location $dirToChangeTo
# do stuff in the new location
Pop-Location   # original location restored

Handling Errors

There's a small problem with any of these methods though: if your script doesn't execute all the way through for any reason, your original location will not be restored. For that you can wrap your entire script in a try/finally block:

Push-Location
try {
    # all your code here
} finally {
    Pop-Location
}

The idea with this is that the code in finally {} should execute always (if a fatal exception occurs, if you stop the script, press CTRL C, etc.), so you can be fairly certain that your original location will get restored. I do this all the time when dealing with the SQLPS module because it always changes the location to the SQLPS drive.

Avoiding this altogether

Of course you really don't have to change the current location. The only place you're using it is where you're finding the zip files, but you could have just passed the current directory into the -Path or -LiteralPath parameter of Get-ChildItem and then you would avoid the need to worry about changing it. I tend to prefer this method personally.

Iterating Directories

The Pipeline

foreach($path in get-childitem -recurse) { #get all subfolders

Get-ChildItem is a pipeline cmdlet, so I would typically recommend you stay with the pipeline, as it tends to be a bit more natural in PowerShell:

Get-ChildItem -Recurse | ForEach-Object {
    $path = $_
}

Getting Directories Only

if ($path.Attributes -eq "Directory") {

You don't need to test for a directory after the call. If you're using PowerShell 3 or higher, you can just do:

Get-ChildItem -Directory -Recurse | ForEach-Object {
    $path = $_  # this will be a directory
}

If you're using v2 you can still keep it in the pipeline but it's a bit more verbose:

Get-ChildItem -Recurse | Where-Object { $_.PSIsContainer } | ForEach-Object {
    $path = $_  # this will be a directory
}

Date Formatting

$date = "$((Get-Date).ToString('yyyy-MM-dd_hh-mm'))"

There's no need to embed this in a string; calling .ToString() will already give you one!

Get-Date has a -Format parameter that takes the same format string as .ToString() so you could just do:

$date = Get-Date -Format 'yyyy-MM-dd_hh-mm'

If you prefer .net objects, you could do it without any cmdlets:

$date = [DateTime]::Now.ToString('yyyy-MM-dd_hh-mm')

12 vs 24 hour clock?

This may be culture dependent and I'm not sure where you are, but on my system 'hh-mm' will give me the 12-hour time, and you're not including AM/PM in your string.

So right now for me it's 8:14 PM and Get-Date -Format 'hh-mm' gives me 08-14. It would give me the same thing in 12 hours though. 'HH-mm' will give me 20-14 which is probably what you want to do here instead of embedding am/pm, but if you did I believe the format code is tt:

Get-Date -Format 'hh-mm tt'

Path Combining

$newpath = [io.path]::combine("$path$date","")

I'm not sure what you're doing here. This is the same thing as:

$newpath = "$path$date"

[System.IO.Path]::Combine() is a way to combine multiple path components without having to concatenate them yourself (and deal with leading/trailing path separators). Am empty string as one of the parameters effectively gets ignored.

It's a great function to know though. There is also the Join-Path cmdlet which does the same thing but in a more "PowerShell" style of course.

Aliases and Item Type

$items = Get-ChildItem -Exclude *.zip | ? {$_.LastWriteTime -lt (Get-date).AddDays(-31) -and -not $_.psIsContainer}

Aliases

? is an alias for Where-Object. Try to avoid aliases in scripts you intend to reuse, especially if they'll be shared or read by others.

Item Type

As above, if you're using PS3+ you can use Get-ChildItem -File to get only files, otherwise you're already doing the right thing with .PSIsContainer.

Test-Path already returns [bool]

if ( $(Try { Test-Path $items } Catch { $false }) ) {

There's no need to use try/catch here; Test-Path already returns $true or $false so you can just use it directly:

if (Test-Path $items) {

else { continue }

StackExchange Code Review Q#110204, answer score: 3