Scripting Games 2013: Event 1 ‘Favorite’ and ‘Not So Favorite’ Submissions

As a follow-up to my previous blog post, I plan to pick out a submission or two or three which stood out as my personal favorite and least favorite and tell you why I think this by pointing pieces of code that was either put together nicely or could have been improved in one way or another. Depending on my time, I will do at least 1 Advanced and 1 Beginner submission for both ‘Favorite’ and ‘Not so Favorite. I’ll start out by listing the code and then discussing it bullet point style to highlight my thoughts. So with that, lets begin this journey through the Event 1 submissions.

One last thing: Please don’t take any of this personal if I negatively critique your code. I will be a little tougher on Advanced than on Beginner because if you are doing the Advanced category, I expect that you know what you are doing. Smile As someone who has competed in the past in the games and also who publishes various scripts and articles online, getting reviewed (and corrected) by your peers is one of the best ways to better yourself as a scripter.

Advanced Category – Not So Favorite Submission

<#
	.SYNOPSIS
		script that can run against local folder structure to move 
		program logs older than specified time to specified location.
		The script will keep the folder structure intact, creating
		subfolders as needed to match the folder structure of the source.
 
	.DESCRIPTION
		Advanced 1 4-29-13
		Dr. Scripto is in a tizzy! It seems that someone has allowed a
		series of application log files to pile up for around two years,
		and they’re starting to put the pinch on free disk space on a
		server.  Your job is to help get the old files off to a new location.
		Actually, this happened last week, too. You might as well create a
		tool to do the archiving.
		The current set of log files are located in C:\Application\Log. There
		are three applications that write logs here, and each uses its own
		subfolder.  For example, C:\Application\Log\App1, C:\Application\Log\OtherApp,
		and C:\Application\Log\ThisAppAlso.  Within those subfolders, the
		filenames are random GUIDs with a .LOG filename extension. Once created
		on disk, the files are never touched again by the applications. Your
		goal is to grab all of the files older than 90 days and move them to
		\\NASServer\Archives ‐ although that path will change from time to time.
		You need to maintain the subfolder structure, so that files from
		C:\Application\Log\App1 get moved to \\NASServer\Archives\App1, and
		so forth.  Make those paths parameters, so that Dr. Scripto can just
		run this tool with whatever paths are appropriate at the time.
		The 90 ‐day period should be a parameter too.  You want to ensure that
		any errors that happen during the move are clearly displayed to whoever
		is running your command. If no errors occur, your command doesn’t need
		to display any output – “no news is good news.”
 
		.EXAMPLE
	Advanced_1.ps1
		This will move the logs from the source folder to the default destination folder
		"\\NASServer\Archives\" with the default time period of 90 days or older.
 
	Advanced_1.ps1 -d RemoteLocation
		This will move the logs from the source folder to the specified destination folder
		with the default time period of 90 days.  The path should have a trailing backslash.
 
	Advanced_1.ps1 -t FilesOlderThanThisNumber
		This will move the logs from the source folder to the default destination folder
		"\\NASServer\Archives\" with the specified time period.  This number should be
		represented by a negative number.
 
	Advanced_1.ps1 -d RemoteLocation -t FilesOlderThanThisNumber
		This will move the logs from the source folder to the specified destination folder
		with the specified time period.
#>
 
Param(
	[alias("d")][string]$RootDestination="\\scanbox\workstation\archive\",
	[alias("t")][string]$TimePeriod="-90"
	)
 
GCI C:\Application\Log -Recurse -Filter "*.log" | `
?{$_.LastWriteTime -lt (get-date).AddDays($TimePeriod)} | `
%{$Dest=$_.Directory.Name;`
If(-not(Test-Path -Path $RootDestination$Dest))`
{NI -itemtype directory $RootDestination$Dest | Out-Null};`
MV -Path $_.FullName -Destination $RootDestination$Dest}
  • Has commented help BUT it is missing the parameters that are in this script. As an advanced function, this has to be complete if it is going to be a re-usable tool.
  • Aliases are everywhere! From GCI to % to ?, this makes it very hard to read
  • Using backticks “`” as a line break is completely unnecessary, especially when you have the pipes “|” that server as a natural break.
  • A lack of any error handling always hurts a script/function
  • Giving the script a better name other than advanced_1.ps1 that better represents what you are using it for. Move-LogFile.ps1 is much better or something similar.
  • Kudos to the default parameter values; just too bad there weren’t a few more so hard coded values were used later in the script. “.Log” and the Source location should have been their own parameters.
  • Would be better as a function for re-usability rather than calling script each time

 

Advanced Category – Favorite Submission

function Move-LogFile {
 
<#
.SYNOPSIS
Move files with a .log extension from the SourcePath to the DestinationPath that are older than Days. 
.DESCRIPTION
Move-LogFile is a function that moves files with a .log extension from the first level subfolders that are specified via
the SourcePath parameter to the same subfolder name in the DestinationPath parameter that are older than the number of days
specified in the Days parameter. If a subfolder does not exist in the destination with the same name as the source, it will
be created. This function requires PowerShell version 3.
.PARAMETER SourcePath
The parent path of the subfolders where the log files reside. Log files in the actual SourcePath folder will not be archived,
only first level subfolders of the specified SourcePath location.
.PARAMETER DestinationPath
Parent Path of the Destination folder to archive the log files. The name of the original subfolder where the log files reside
will be created if it doesn't already exist in the Destination folder. Destination subfolders are only created if one or more
files need to be archived based on the days parameter. Empty subfolders are not created until needed.
.PARAMETER Days
Log files not written to in more than the number of days specified in this parameter are moved to the destination folder location.
.PARAMETER Force
Switch parameter that when specified overwrites destination files if they already exist.
.EXAMPLE
Move-LogFile -SourcePath 'C:\Application\Log' -DestinationPath '\\NASServer\Archives' -Days 90
.EXAMPLE
Move-LogFile -SourcePath 'C:\Application\Log' -DestinationPath '\\NASServer\Archives' -Days 90 -Force
#>
 
    [CmdletBinding()]
    param (
        [string]$SourcePath = 'C:\Application\Log',
        [string]$DestinationPath = '\\NASServer\Archives',
        [int]$Days = 90,
        [switch]$Force
    )
 
    BEGIN {        
        Write-Verbose "Retrieving a list of files to be archived that are older than $($Days) days"
        try {
            $files = Get-ChildItem -Path (Join-Path -Path $SourcePath -ChildPath '*\*.log') -ErrorAction Stop |
                     Where-Object LastWriteTime -lt (Get-Date).AddDays(-$days)
        }
        catch {
            Write-Warning $_.Exception.Message
        }
 
        $folders = $files.directory.name | Select-Object -Unique
        Write-Verbose "A total of $($files.Count) files have been found in $($folders.Count) folders that require archival"
    }
 
    PROCESS {
        foreach ($folder in $folders) {
 
            $problem = $false
            $ArchiveDestination = Join-Path -Path $DestinationPath -ChildPath $folder
            $ArchiveSource = Join-Path -Path $SourcePath -ChildPath $folder
            $ArchiveFiles = $files | Where-Object directoryname -eq $ArchiveSource
 
            if (-not (Test-Path $ArchiveDestination)) {
                Write-Verbose "Creating a directory named $($folder) in $($DestinationPath)"
                try {
                    New-Item -ItemType directory -Path $ArchiveDestination -ErrorAction Stop | Out-Null
                }
                catch {
                    $problem = $true
                    Write-Warning $_.Exception.Message
                }
            }
 
            if (-not $problem) {
                Write-Verbose "Archiving $($ArchiveFiles.Count) files from $($ArchiveSource) to $($ArchiveDestination)"
                try {
                    If ($Force) {
                        $ArchiveFiles | Move-Item -Destination $ArchiveDestination -Force -ErrorAction Stop
                    }
                    Else {
                        $ArchiveFiles | Move-Item -Destination $ArchiveDestination -ErrorAction Stop
                    }
                }
                catch {
                    Write-Warning $_.Exception.Message
                }
            }
 
        }
    }
 
    END {
        Remove-Variable -Name SourcePath, DestinationPath, Days, Force, files, folders, folder,
        problem, ArchiveDestination, ArchiveSource, ArchiveFiles -ErrorAction SilentlyContinue
    }
 
}
  • Great use of commented help to include all parameters and nice examples
  • Use of a function (with proper naming convention) which is great!
  • Using Write-Verbose and making sure to use [cmdletbinding()] allows the user running the function to specify the –Verbose parameter to add more information to the console when being run.
  • Great use of error handling to deal with anything that comes up
  • I wish that there would have been a parameter for Filter so it could handle other file extensions.
  • Makes good use of the Begin,Process and End blocks; these were not technically needed as none of the parameters allowed for the use of the pipeline.
  • I like the use of Join-Path to combine the folder paths; makes the code look cleaner
  • Also liked the use of ‘*\*.log’ in the Get-ChildItem command to get all of the files; clever

Beginner Category – Not So Favorite Submission

Get-ChildItem -Path 'C:\Application\Log' -Recurse | Where-Object {
$_.LastWriteTime -lt (Get-Date).AddDays(-90)
} | Move-Item -Force -Destination '\\NASServer\Archives'

Note: This is a one-liner that I intentionally broke up so I can all fit without much scrolling.

  • This assumes that there will only be .Log files; should have used –Filter to grab only the .log files. Never assume with a file search.
  • No appropriate destination folders are created to archive each of the log files from their specified source directory.
  • Because the proper destination directory doesn’t exist, the logs are all bundled together in one folder.
  • Commented help would always be great; even in a beginner event. The sooner you start adding the help, the better off you will be.
  • I do like that full cmdlet names were used instead of aliases

 

Beginner Category – Favorite Submission

<#
.SYNOPSIS
    Moves application log files that are older than 90 days to \\NASServer\Archives.
.DESCRIPTION
    This script was originally written for Event 1 of the 2013 Scripting Games (Beginners Track).
.EXAMPLE
    C:\Scripts\Move-LogFiles.ps1  
#>
 
$files = Get-ChildItem –Path  “C:\Application\Log” -Filter *.log  –Recurse | Where-Object -FilterScript {$_.CreationTime –lt (Get-Date).AddDays(-90)} 
 
foreach($file in $files)
{
    if(!(Test-Path -LiteralPath "\\NASServer\Archives\$($file.Directory.Name)"))
    {
            New-Item -Path "\\NASServer\Archives\$($file.Directory.Name)" -ItemType Directory -ErrorAction Stop | Out-Null
    }
 
    Move-Item -Path $file.FullName -Destination "\\NASServer\Archives\$($file.Directory.Name)\$($file.Name)"
}
 
  • Kudos for the commented help in this script. This makes it easier for someone to pick and run Get-Help against to better understand how this works.
  • Good use of –Filter against the .log extension
  • I like that there were no vbscript style concatenation happening with the folder paths
  • Per the submission requirement, using Out-Null with New-Item to create each directory suppresses the object output that occurs when creating anything with the New-Item cmdlet.
  • Clean code that makes it easy to read and understand what is happening. Could have done a line break at the Where-Object, but still readable.

That’s it for Event 1. Thanks again to everyone who has competed this year and submitted their entries! Everyone here has done incredible work and I am truly looking forward to seeing what is submitted for the future events! Keep it up!

About Boe Prox

Microsoft Cloud and Datacenter MVP working as a SQL DBA.
This entry was posted in 2013 Scripting Games Judges Notes, powershell, Scripting Games 2013 and tagged , , . Bookmark the permalink.

5 Responses to Scripting Games 2013: Event 1 ‘Favorite’ and ‘Not So Favorite’ Submissions

  1. GK says:

    Could you elaborate on the use of ‘*\*.log’ ? I’m not seeing the problem it solves…

  2. chris says:

    That’s my advanced script and my snippet of beginner. I know someone who won’t use backticks again!

  3. Scott says:

    I wrote the ‘not so favorite’ beginner one-liner. Thanks for your criticism.

    I disagree on your first two comments. Those points were considered during creation. The problem clearly states that all files are named .log. The filter was included to prevent extraneous files from being involved. The problem also states there are three applications which create logs in the folder and explicitly gives the literal path names of the folders where the log files are stored.

    The folders issue: I cleaned my submission too thoroughly. Originally I had noted that the first time the one-liner is run we needed to manually ensure the folders were there.

    Personally, I would not use Powershell for this. I would use robocopy. It will create any folder structure needed and can filter by file age.

    Finally, I will include the help bits for future submissions.

    • Boe Prox says:

      Thank you for your comment.
      You say that you disagree with my first two comments which is absolutely fine. I want to make sure to address both of these items.

      1: Based on the requirements, while it says that the .log files do reside on the specified folders, it doesn’t explicitly say that those are the only files. With that, we really cannot assume that those are the only files (or even the only sub folders) underneath the C:\Application\Log folder. Even if the event said that there were no sub folders and the only files that would exists would be .log files, it is a good practice regardless to use the -Filter parameter to more efficiently find files that match a certain string (in this case, the .log). Remember that the games are more about learning than anything else.

      2: I am assuming that you are talking about my comment about not having created the destination folders that mirror what was at the source. I do like the idea of using robocopy to handle this and wouldn’t have had an issue with that (though I hear most of the community was downvoting folks who did this) as it would be innovative and accomplish the requirements of this event. Another option if you wanted to keep the one-liner approach would have been to use Move-Item with something like this:

      Move-Item -Force -Destination {mkdir (Join-Path “\\NASServer\Archives\” (Split-Path -Path $_.Directory -Leaf)) -Force}

      Using -Force on New-Item with a directory will still output an object but doesn’t overwrite any of the sub items underneath it.

      Hope this helps…

  4. Pingback: Scripting Games 2013: Event 1 ‘Favorite’ and ‘Not So Favorite’ Submissions | PowerShell.org

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s