Help converting script to a function

This topic contains 7 replies, has 2 voices, and was last updated by Profile photo of Sam Boutros Sam Boutros 1 year, 9 months ago.

  • Author
    Posts
  • #22926
    Profile photo of Vince Bailey
    Vince Bailey
    Participant

    Hi, newbie Powershell person needing some assistance...

    I've written a powershell script that checks all my servers for disk space (yep, the usual first powershell script), and compares it to the last time the script was run. This all works OK.

    I am now trying to covert this to functions, and I have run across a problem.

    The first function accepts a list of computernames and outputs the disk space results as custom objects. This gets exported to a an XML file so that it can be used for comparison the next time the script is run.

    $ServerList = "CN1", "CN2", "CN3"
    
    $CurrentDriveStats = Get-DriveStats -ComputerName $ServerList
    
    $CurrentDriveStats | Export-Clixml "C:\temp\statfile.xml"
    

    My object looks like this:

    PS C:\temp> $CurrentDriveStats | gm
    
    
       TypeName: System.Management.Automation.PSCustomObject
    
    Name                MemberType   Definition                             
    ----                ----------   ----------                             
    Equals              Method       bool Equals(System.Object obj)         
    GetHashCode         Method       int GetHashCode()                      
    GetType             Method       type GetType()                         
    ToString            Method       string ToString()                      
    ComputerName        NoteProperty System.String ComputerName
    DriveLetter         NoteProperty System.String DriveLetter           
    DriveSizeInGB       NoteProperty System.Int32 DriveSizeInGB          
    DriveSpaceInGB      NoteProperty System.Int32 DriveSpaceInGB         
    DriveSpaceInPercent NoteProperty System.Int32 DriveSpaceInPercent  
    

    This all works OK.

    Next time it is run, the XML file is imported so that it can be used as a comparison:

    $PreviousDriveStats = Import-Clixml "C:\temp\statfile.xml"
    

    I then pass the previous stats and the current stats to a comparison function (which matches up the previous and current objects and outputs new objects):

    Compare-DriveStats $PreviousDriveStats, $CurrentDriveStats | Format-Table -AutoSize
    
    Function Compare-DriveStats ($PreviousDriveStats, $CurrentDriveStats)
    {
    
        Begin {}
    
        Process {
    
             ForEach ($C in $CurrentDriveStats) {
    
                ForEach ($P in $PreviousDriveStats) {
    
                    # Write-Output $P.ComputerName, $P.DriveLetter, $C.ComputerName, $C.DriveLetter
    
                    If (($P.ComputerName -eq $C.ComputerName) -And ($P.DriveLetter  -eq $C.DriveLetter)) {
    
                        #Build hash table of results - NB [Ordered] omitted as PS3+ only
                        $Props = @{'ComputerName'                = $P.ComputerName;
                                   'DriveLetter'                 = $P.DriveLetter;
                                   'DriveSizeInGB'               = $P.DriveSizeInGB;
                                   'PreviousDriveSpaceInGB'      = $P.DriveSpaceInGB;
                                   'PreviousDriveSpaceInPercent' = $P.DriveSpaceInPercent;
                                   'CurrentDriveSpaceInGB'       = $C.DriveSpaceInGB;
                                   'CurrentDriveSpaceInPercent'  = $C.DriveSpaceInPercent}
    
                        #Create object to be returned
                        $Obj = New-Object -TypeName PSObject -Property $Props
    
                        #Return the object
                        Write-Output $Obj
             
                    }
    
                }
    
            }
        }
    
    
        End {}
    
    }
    

    Now this works as a script but not as a function. I've been looking at this for a few days on and off and I can't work out if I have made a silly mistake or I have a not understood a concept somewhere.

    I would be really grateful if someone could point out where I am going wrong – maybe I am attacking the problem the wrong way?

    Thanks and regards,

  • #22927
    Profile photo of Sam Boutros
    Sam Boutros
    Participant

    Having not looked at the rest of your code, to have the function return the $Obj, just drop it in the pipeline – simply remove 'Write-Output' after the '#Return the Object' comment, keeping '$obj' on a line by itself..

  • #22929
    Profile photo of Vince Bailey
    Vince Bailey
    Participant

    Yes, that makes sense. But I think something is going wrong with passing the objects in.

    If I uncomment the Write-Output line I would expect to see something like:

    CN1, C$, CN1, C$
    CN1, C$, CN1, D$
    CN1, C$, CN2, C$
    CN1, D$, CN2, D$
    ...

    But I don't get any output at all. I should have said all of this in my question – my bad.

  • #22931
    Profile photo of Sam Boutros
    Sam Boutros
    Participant

    What's Get-DriveStats ?

  • #22972
    Profile photo of Vince Bailey
    Vince Bailey
    Participant

    Get-DriveStats:

    Function Get-DriveStats
    {
    
        [CmdletBinding(SupportsShouldProcess = $True,
        #PS3+ only     PositionalBinding = $False,
                       ConfirmImpact = 'Low')]
    
        Param(
            #Computername(s) to be checked for disk space statistics
            [Parameter(Mandatory = $True,
                       ValueFromPipeline = $True,
                       ValueFromPipelineByPropertyName = $True,
                       Position = 0)]
            [String[]]$ComputerName )
    
        Begin {}
    
        Process {
    
            #Iterate for every ComputerName supplied to function
            ForEach ($C in $ComputerName) {
    
                #Get fixed drive statistics for current ComputerName
                $WmiObject = Get-WmiObject Win32_LogicalDisk -ComputerName $C -Filter "DriveType = 3"
    
                #Iterate for every object returned (one object per fixed disk found in target computer
                ForEach ($W in $WmiObject) {
    
                    #Build variables with required statistics
                    $DriveLetter = $W.DeviceID
                    $DriveSizeInGB = [Int]($W.Size / 1GB)
                    $DriveSpaceInGB = [Int]($W.FreeSpace / 1GB)
                    $DriveSpaceInPercent = [Int](($DriveSpaceInGB / $DriveSizeInGB) * 100)
    
                    #Build hash table of results - NB [Ordered] omitted as PS3+ only
                    $Props = @{'ComputerName'        = $C;
                               'DriveLetter'         = $DriveLetter;
                               'DriveSizeInGB'       = $DriveSizeInGB;
                               'DriveSpaceInGB'      = $DriveSpaceInGB;
                               'DriveSpaceInPercent' = $DriveSpaceInPercent}
    
                    #Create object to be returned
                    $Obj = New-Object -TypeName PSObject -Property $Props
    
                    #Return the object
                    Write-Output $Obj
                    }
                }
            }
    
        End {}
    
    }
    
  • #22974
    Profile photo of Sam Boutros
    Sam Boutros
    Participant

    Function needs some changes:

    #Requires -Version 2.0
    
    Function Get-DriveStats
    {
     
        [CmdletBinding(SupportsShouldProcess = $True,
        #PS3+ only     PositionalBinding = $False,
                       ConfirmImpact = 'Low')]
     
        Param(
            #Computername(s) to be checked for disk space statistics
            [Parameter(Mandatory = $false,
                       ValueFromPipeline = $True,
                       ValueFromPipelineByPropertyName = $True,
                       Position = 0)]
            [String[]]$ComputerName = $env:COMPUTERNAME
        )
     
        Begin {}
     
        Process {
     
            # Declase output object as empty array
            $Disks = @()
    
            # Iterate for every ComputerName supplied to function
            foreach ($Computer in $ComputerName) {
     
                # Get fixed drive statistics for current Computer
                try { $RawDisks = Get-WmiObject Win32_LogicalDisk -ComputerName $Computer -Filter "DriveType = 3" } catch { throw }
                
                # Iterate for every object returned (one object per fixed disk found in target computer
                $RawDisks | % {
     
                    # Build hash table of results
                    $Props = @{
                        ComputerName     = $Computer
                        DriveLetter      = $_.DeviceID
                        TotalSpaceInGB   = [Math]::Round($_.Size/1GB, 2)
                        FreeSpaceInGB    = [Math]::Round($_.FreeSpace/1GB, 2)
                        UsedSpaceInGB    = [Math]::Round(($_.Size - $_.FreeSpace)/1GB, 2)
                        FreeSpacePercent = [Math]::Round($_.FreeSpace / $_.Size * 100, 2)
                    }
     
                    $Disks += New-Object -TypeName PSObject -Property $Props
     
                } # foreach $RawDisks
    
            } # foreach $Computer
    
            # Return the output object - using select instead of [ordered] hash table to be PS2 compliant
            $Disks | select ComputerName,DriveLetter,TotalSpaceInGB,FreeSpaceInGB,UsedSpaceInGB,FreeSpacePercent
    
        } # Process
     
        End {}
     
    }
    

    Then it can be used like:

    Get-DriveStats | FT -Auto 
    

    To save results to CSV:

    Get-DriveStats | Export-Csv ".\$env:COMPUTERNAME-Drives1.csv" -NoType
    

    I recommend getting in the habit of saving objects to XML instead of CSV if the intent is to compare later on. XML can be used to properly save complex objects where CSV cannot:

    Get-DriveStats | Export-Clixml ".\$env:COMPUTERNAME-Drives1.xml"
    

    To compare, simply use the compare-object cmdlet:

    $Difference = Compare-Object (Get-DriveStats) (Import-Clixml ".\$env:COMPUTERNAME-Drives1.xml") -PassThru -Property FreeSpaceInGB
    if ($Difference) {
        $Difference | FT -AutoSize
    } else {
        "No drive size changes detected"
    }
    
  • #23023
    Profile photo of Vince Bailey
    Vince Bailey
    Participant

    Many thanks. I can see that version makes more sense – build an array of objects and return the array rather than return one object at a time.

    Your guidance is much appreciated.

  • #23025
    Profile photo of Sam Boutros
    Sam Boutros
    Participant

    sure 🙂

You must be logged in to reply to this topic.