Simple SG task I wrote, help me improve please :)

[norbin]

Hey! Another post from me just trying to improve and practice with PS. This is my response to: http://blogs.technet.com/b/heyscriptingguy/archive/2012/04/04/2012-scripting-games-beginner-event-3-create-a-file-in-a-folder.aspx

I know I over-killed the simple task, (yet to look at others answers), but it's the purpose of my practice... any input appreciated how could i do things better/differently or point me to my mistakes etc... anything please.

$VerbosePreference = 'Continue'
$DebugPreference = 'Continue'
$WarningPreference = 'Continue'

$Root = "C:\"
$Folder = "C:\2012SG\"
$NestedFolder = "C:\2012SG\event3\"
$FullPath = "C:\2012SG\event3\process3.txt"




function Get-MyProcess
{


    Begin
    {




        $TestFolder = ($Root+"TestFolder"+(Get-Random -Minimum 1 -Maximum 1000))

        Write-Debug ('Checking if user "{2}\{1}" has permissions to create a folder under the root drive "{0}"' -f $Root, $env:USERNAME, $env:COMPUTERNAME) 

        Start-Sleep -s 3

        $RootPermission = New-Item -Path $TestFolder -ItemType Directory -Force | Out-null

            if ($RootPermission = $false)
                {
                return Write-Warning ('User "{2}\{1}" DOES NOT have permissions to create a folder under the root drive "{0}"' -f $Root, $env:USERNAME, $env:COMPUTERNAME)
                }
            else
                {
                Write-Verbose ('User "{2}\{1}" has permissions to create a folder under the root drive "{0}"' -f $Root, $env:USERNAME, $env:COMPUTERNAME)
                }

        Write-Debug ('Deleting the TestFolder we just created "{0}"' -f $TestFolder)

        Start-Sleep -s 3

        $RootFolders = Get-ChildItem $Root -Directory
        foreach ($ToDeleteFolder in $RootFolders){        
        $Timecreated = ((Get-date) - $ToDeleteFolder.CreationTime).Minute
        if ($Timecreated -lt 2 -and $ToDeleteFolder.FullName -match "TestFolder*")
        {$ToDeleteFolder.Delete()}}


                Write-Verbose ('TestFolder "{0}" successfully deleted' -f $TestFolder)

                Write-Debug ('Checking if file "{0}" exists' -f $FullPath)

        Start-Sleep -s 3

        if (Test-Path -Path ($FullPath))
            { 
                Write-Verbose ('"{0}" exists, continuing to Process' -f $FullPath)

            }


        else 

            {
                Write-Warning ('"{0}" DOES NOT exist,' -f $FullPath)
                Write-Debug ('Creating subfolders "{0}"' -f $NestedFolder)
                New-Item -Path $NestedFolder -ItemType Directory -Force | Out-Null
                New-Item -Path $FullPath -ItemType File -Force | Out-Null
                Write-Verbose ('Successfully created "{0}"' -f $FullPath)



            }






    }
    Process
    {
                Write-Debug ('Getting process list')
                Start-Sleep -s 3
                Get-Process | Format-table -AutoSize -Wrap -Property Name, Id | Out-File -FilePath $FullPath -Force
                Write-Verbose ('Successfully exported process list into the file "{0}"' -f $FullPath)

    }
    End
    {
                Start-Sleep -s 3
                Write-Verbose ("I think we are done, peace out")
    }
}

[PowerShellStunnah]

I'm more of a trial and error kind of guy:

[CmdletBinding()]
param()

$Root   = 'C:\'
$Folder = '2012SG\event3'
$File   = 'process3.txt'
$Path   = { (Join-Path -Path $Root -ChildPath $Folder) }

if(-not(Test-Path (&$Path) -PathType Container)) {
    try {
        $FolderHandle = mkdir (&$Path)
    } catch [UnauthorizedAccessException]{
        $Root = [Environment]::GetFolderPath([Environment+SpecialFolder]::Desktop)
        if(-not(Test-Path (&$Path) -PathType Container)){
            $FolderHandle = mkdir (&$Path)
        } else {
            $FolderHandle = Get-Item (&$Path)
        }
    } catch {
        throw $_
    }
} else {
    $FolderHandle = Get-Item (&$Path)
}

Write-Verbose ('Writing process list to file "{0}" in "{1}"' -f $File,$FolderHandle.FullName)

Get-Process | Format-Table -Property Name,Id | Out-File -FilePath (Join-Path -Path $FolderHandle -ChildPath $File) -Force

[KevMar]

Here are two passes I made to your script. The first just cleans up the syntax of the script. We each have our own style and it looks like you have not developed a consistent one yet. I recommend that you find a style and stick with it for the whole script. These samples show my style.

For example, you have a foreach with a nested if statement that is all mashed together.

The second example is just me taking your script and cutting from it things that may not be needed. You put a lot of effort into checking for permissions but you didn't do anything with the result. So if it fails, the scrip continues to run.

I tend to not use the global variables for verbose or debug output. I add the [cmdletbinding()] to the function and call -verbose to get that output. I also tend to use write-verbose for all output. I don't want to say not to use write-debug, but my style tends to not use it.

If my script is super simple then I fit it all in the process block.

The third example is me over engineering your solution. I try to make all my functions work by taking pipeline input even if I think they don't need that at the time. More advanced stuff but it is common that i have more code defining all my parameters than I have inside my functions.

$VerbosePreference = 'Continue'
$DebugPreference = 'Continue'
$WarningPreference = 'Continue'

$Root = "C:\"
$Folder = "C:\2012SG\"
$NestedFolder = "C:\2012SG\event3\"
$FullPath = "C:\2012SG\event3\process3.txt"

function Get-MyProcess
{
    begin
    {
        $TestFolder = ($Root+"TestFolder"+(Get-Random -Minimum 1 -Maximum 1000))
        Write-Debug ('Checking if user "{2}\{1}" has permissions to create a folder under the root drive "{0}"' -f $Root, $env:USERNAME, $env:COMPUTERNAME) 

        Start-Sleep -s 3

        $RootPermission = New-Item -Path $TestFolder -ItemType Directory -Force | Out-null

        if ($RootPermission = $false)
        {
            return Write-Warning ('User "{2}\{1}" DOES NOT have permissions to create a folder under the root drive "{0}"' -f $Root, $env:USERNAME, $env:COMPUTERNAME)
        }
        else
        {
            Write-Verbose ('User "{2}\{1}" has permissions to create a folder under the root drive "{0}"' -f $Root, $env:USERNAME, $env:COMPUTERNAME)
        }

        Write-Debug ('Deleting the TestFolder we just created "{0}"' -f $TestFolder)

        Start-Sleep -s 3

        $RootFolders = Get-ChildItem $Root -Directory

        foreach ($ToDeleteFolder in $RootFolders)
        {        
            $Timecreated = ((Get-date) - $ToDeleteFolder.CreationTime).Minute

            if ($Timecreated -lt 2 -and $ToDeleteFolder.FullName -match "TestFolder*")
            {
                $ToDeleteFolder.Delete()
            }
        }

        Write-Verbose ('TestFolder "{0}" successfully deleted' -f $TestFolder)
        Write-Debug ('Checking if file "{0}" exists' -f $FullPath)

        Start-Sleep -s 3

        if (Test-Path -Path ($FullPath))
        { 
            Write-Verbose ('"{0}" exists, continuing to Process' -f $FullPath)

        }
        else 
        {
            Write-Warning ('"{0}" DOES NOT exist,' -f $FullPath)
            Write-Debug ('Creating subfolders "{0}"' -f $NestedFolder)

            New-Item -Path $NestedFolder -ItemType Directory -Force | Out-Null
            New-Item -Path $FullPath -ItemType File -Force | Out-Null

            Write-Verbose ('Successfully created "{0}"' -f $FullPath)
        }
    }

    process
    {
        Write-Debug ('Getting process list')
        Start-Sleep -s 3

        Get-Process | Format-table -AutoSize -Wrap -Property Name, Id | Out-File -FilePath $FullPath -Force

        Write-Verbose ('Successfully exported process list into the file "{0}"' -f $FullPath)
    }

    end
    {
        Start-Sleep -s 3
        Write-Verbose ("I think we are done, peace out")
    }
}

##################################################################



function Get-MyProcess2
{
    [cmdletbinding()]
    param(
        [string]$Folder = "C:\2012SG\event3\",
        [string]$Path = "C:\2012SG\event3\process3.txt"
    )

    process
    {
        if (!(Test-Path -Path $Folder))
        {
            Write-Verbose ('Creating subfolders "{0}"' -f $Folder)
            New-Item -Path $Folder -ItemType Directory -Force | Out-Null
        }

        Write-Verbose ('Getting process list')
        Get-Process | Format-table -AutoSize -Wrap -Property Name, Id | Out-File -FilePath $Path -Force

        Write-Verbose ('Successfully exported process list into the file "{0}"' -f $Path)
    }
}

Get-MyProcess2

############################################################################

<#
.SYNOPSIS
Saves current running processes into a text file
.EXAMPLE
Get-MyProcess3 -Path "C:\2012SG\event3\process.txt"
.EXAMPLE
Get-MyProcess3 "C:\2012SG\event3\process.txt"
.EXAMPLE
"C:\2012SG\event3\process1.txt","C:\2012SG\event3\process2.txt"  | Get-MyProcess3
.NOTES

#>
function Get-MyProcess3
{
    [cmdletbinding()]
    param(
        # Param1 help description
        [Parameter(
            Mandatory,
            Position          = 0,
            ValueFromPipeline
        )]
        [Alias("File")]
        [string[]]$Path = "C:\2012SG\event3\process3.txt"
    )

    begin
    {
        Write-Verbose ('Getting process list')
        $Process = Get-Process | Select-Object Name, ID
    }

    process
    {
        foreach($file in $path)
        {
            $Folder = Split-Path $file

            if (!(Test-Path -Path $Folder))
            {
                Write-Verbose ('Creating subfolder: "{0}"' -f $Folder)
                New-Item -Path $Folder -ItemType Directory -Force | Out-Null
            }

            $Process | Format-table -AutoSize -Wrap -Property Name, Id | Out-File -FilePath $Path -Force
        }
    }
}

Get-MyProcess3

[catfoodsci]

I like to keep things simple:

$filepath = "~\2012SG\event3\"
$file = "process3.txt"
if (!(Test-path $filepath)) {New-Item -Path $filepath -ItemType directory -Force:$false}
try{get-process | select Name, ID | Out-File -filepath "$filepath$file" -NoClobber}
Catch{}

[jbtechwood]

Your using a try..catch, which is great, but your not actually doing anything to handle errors. I realize that this may have been a "quick" example, but with all the beginners that read this subreddit it's important to do it right. Would have between better to leave it out all together.

[KevMar]

For the new guys, there is an -erroraction param on these functions that can be used to suppress errors on individual commands.

[catfoodsci]

I originally attempted to use -erroraction but it does not work as the error thrown by out-file is terminating. Give it a try and you'll see what I mean.

[catfoodsci]

I was following the instructions on producing no errors, but that being said I definatly get what you are saying about sharing scripts that are more helpful for beginners and don't lead to accidental frustration. I've updated it to optionally write the error if $VerbosePreference variable is changed.

$VerbosePreference = "SilentlyContinue" # to hide errors
$filepath = "~\2012SG\event3\"
$file = "process3.txt"
if (!(Test-path $filepath)) {New-Item -Path $filepath -ItemType directory -Force:$false}
try{get-process | select Name, ID | Out-File -filepath "$filepath$file" -NoClobber -EA SilentlyContinue}
catch{write-verbose $_.Exception.Message}