Should I or Shouldn't I - use functions inside a ps module

PowerShell Help
1

I have a module that I use daily. I am modifying it from its current state, csv input only, to being able to accept input for a single item as well. In doing this I am switching on parameter sets, which means duplicating the same bit of script for each switch option.

I had thought that it might be better to create a function and then call that function for each switch selection, rather than duplicating it in the script for each one.

However in one of my go to guides - PowerShell Toolmaking in a month of lunches, the authors make it very clear that you shouldn’t do this except in testing (p38).

So my question is this. Is there ever a case where defining and then consuming a function inside a ps module is an acceptable or even good idea?

@donj

2

Don doesn’t always peruse the forums, but…

the authors make it very clear that you shouldn't do this except in testing (p38).

I don’t have the book handy to understand what you are referencing, however, a Powershell module is a compilation of functions that are referenced as cmdlets. Module authors will sometimes have Private (helper functions) and Public functions defined a module. If you are duplicating the same code over and over again, then there is most likely a better design, but it’s hard for us to provide input without seeing the code.

3

It could be that I misunderstood that part… They are talking about creating a function and then testing it by defining it within the script and then running it in the same script. They use this as a launch pad to discuss modules, but nowhere else do they talk about defining/running functions within the module.

From the book: “…Neither dot sourcing nor having the script run the function is appropriate outside of testing. What would be better is to provide a defined way to load the script (and it’s functions) into memory, allow those functions to be run on demand, and also provide a way to quickly and easily remove them all from memory if needed. That way is called a module,…”

The script is below. I was leaning towards taking the script from line #70 - #85 and defining it as a function just after line #65. Then calling it here at line #70 and again at line #141 in place of having the code written twice.

[pre]

function Create-ForwardGroups
{

<#
.SYNOPSIS
Moves ad account to DisabledUsers OU. Pauses to allow sync to be disabled when using csv input. Creates unified groups for forwarding of email.
.SYNTAX
Create-ForwardGroups [-Path] [-MyMail]
.DESCRIPTION
Create-ForwardGroups requires access to AD and O365. Using these connections it moves the user account to the ‘Disabled Users’ OU when using csv input. Pauses for 30 minutes to allow time for sync to O365 to take place.
After 30 minutes a unified group will be created as a forwarder and an email will be sent to the recipient. Creation of the forward group for a single individual expects the AD account to already be in the ‘Disabled Users’ OU and will be processed immediately.
.PARAMETER Path
The path where the input .csv file is located. A folder named forwarding is expected.
.PARAMETER MyMail
The email address of the person executing the script
.PARAMETER Mail
The email address of the employee whose mail is being forwarded.
.PARAMETER Sam
The legacy logon name for the employee whose mail is being forwarded (e.g. jdoe, nc10075)
.PARAMETER Name
The friendly name for the employee whose mail is being forwarded. (e.g. John Doe, Jane Doe), this name will be entered with quotes. “Jane Doe”, “John Doe”
.PARAMETER Recipient
The email address for the person who will be receiving the forwarded mail.
.EXAMPLE
Create-ForwardGroups -Path c:\temp\forwarding\xxx.csv -MyMail john.doe@contoso.com
.EXAMPLE
Create-ForwardGroups -MyMail jane.hancock@contoso.com -Name “John Doe” -Sam jdoe -Mail john.doe@contoso.com -Recipient Jane.Manager@contoso.com
#>

[CmdletBinding()]
param (
[Parameter(Mandatory = $true)]
[string]$MyMail,

[Parameter(Mandatory = $true,
ParameterSetName = “CsvInput”)]
[string]$Path,

[Parameter(Mandatory = $True,
ParameterSetName = “Individual”)]
[string]$Mail,
[string]$Sam,
[string]$Recipient,
[string]$Name
)

$EmailBody = @"
<html>
<body style=“font-family:arial;font-size:13px”>
<p>Email forwarding for VarName has been created. We are now using Office 365 groups to handle email forwarding for terminated employees, it will work differently than what has been done before.

<p>When you no longer need the email forwarded to you, you can delete the group using the instructions below. NOTE: O365 groups are automatically set to expire after 180 days. Beginning at 150 days you will receive several email notifications, spaced across a 30 day period, asking if you want to renew the group. If you do not renew the group, (by clicking the renew button in the automated email message) it will be deleted. It can be recovered if requested within 30 days, but is permanently deleted after that.</p>

<p>To delete the group:</p>

  1. Expand the Groups section (located in the left navigation pane) in Outlook and select the desired #Forwarder group.
  2. On the ribbon bar choose `'Group Settings -> Edit Group`'.
  3. Select `'Delete Group`' from the bottom left corner of the dialog box.
  4. In the Pop-Up dialog box, check the box `'I understand that all group content will be deleted`'. This ONLY refers to items in the group. Items that were forwarded to your inbox will not be affected.
</body> </html> "@

switch ($PSCmdlet.ParameterSetName)
{
“Individual”{

$displayname = “#Forwarder-” + $Name
$alias = “#” + $Sam + “forwarder”

new-unifiedgroup -DisplayName $displayname -alias $alias -primarysmtpaddress $Mail -accesstype private -autosubscribenewmembers -requiresenderauthentication:$false
set-unifiedgroup -Identity $displayname -hiddenfromaddresslistsenabled:$true -UnifiedGroupWelcomeMessageEnabled:$false
Add-UnifiedGroupLinks -Identity $displayname -LinkType Members -Links $Recipient
Add-UnifiedGroupLinks -Identity $displayname -LinkType Owners -Links $Recipient
Remove-UnifiedGroupLinks -Identity $displayname -LinkType Owners -Links $MyMail -confirm:$false
Remove-UnifiedGroupLinks -Identity $displayname -LinkType Members -Links $MyMail -confirm:$false

$EmailBody = $EmailBody.Replace(“VarName”, $Name)

Send-MailMessage -to $Recipient -Subject “DO NOT REPLY: Email Forwarding for $Name” -From ‘TerminationProcess@contoso.com’ -Body $emailbody -bodyashtml -SmtpServer smtp.Contoso.com -port 25

$EmailBody = $EmailBody.Replace($Name, “VarName”)
}

“CsvInput”{

$sheets = get-ChildItem “$Path\forwarding*.csv”
$csvpath = "$Path\Forwarding"

$NewDN = “OU=Disabled Users,DC=Contoso,DC=COM”

foreach ($item in $sheets)
{

$filename = $item.name
$filename

$tasks = Import-csv “$csvpath$filename”

foreach ($i in $tasks)
{
$filter = $i.cmail

$dn = get-aduser -filter ‘Mail -like $filter’ -properties distinguishedName | select -ExpandProperty distinguishedName
Move-ADObject $dn -TargetPath $NewDN
}
}

$x = 30 * 60
$length = $x / 100

while ($x -gt 0)
{
$min = int
$text = " " + $min + " minutes " + ($x % 60) + " seconds left"
Write-Progress “Pausing Script” -status $text -perc ($x/$length)
start-sleep -s 1
$x–
}

foreach ($item in $sheets)
{
$filename = $item.name
$filename

$tasks = Import-Csv “$csvpath$filename”

foreach ($i in $tasks)
{
$mail = $i.cmail
$sam = $i.sam

$Name = $i.firstname + " " + $i.lastname
$Recipient = $i.forwardtoa

$displayname = “#Forwarder-” + $name
$alias = “#” + $sam + “forwarder”

new-unifiedgroup -DisplayName $displayname -alias $alias -primarysmtpaddress $mail -accesstype private -autosubscribenewmembers -requiresenderauthentication:$false
set-unifiedgroup -Identity $displayname -hiddenfromaddresslistsenabled:$true -UnifiedGroupWelcomeMessageEnabled:$false
Add-UnifiedGroupLinks -Identity $displayname -LinkType Members -Links $Recipient
Add-UnifiedGroupLinks -Identity $displayname -LinkType Owners -Links $Recipient
Remove-UnifiedGroupLinks -Identity $displayname -LinkType Owners -Links $MyMail -confirm:$false
Remove-UnifiedGroupLinks -Identity $displayname -LinkType Members -Links $MyMail -confirm:$false

$displayname

$EmailBody = $EmailBody.Replace(“VarName”, $name)

Send-MailMessage -to $members -Subject “DO NOT REPLY: Email Forwarding for $Name” -From ‘TerminationProcess@contoso.com’ -Body $emailbody -bodyashtml -SmtpServer smtp.contoso.com -port 25

$EmailBody = $EmailBody.Replace($Name, “VarName”)
}
}
}
}

}

[/pre]

 

 

4

Yeah, you could turn that into a mini-function. Make sure you specify parameters properly, though. It’s not a terrible thing now to be breaking scope, but if you don’t take care to properly declare parameters as a general rule, you’ll shoot yourself in the foot later. It’s a nightmare trying to debug long scripts / large modules when you’ve got to deal with multiple functions working with the same value that isn’t declared by that function and instead exists somewhere outside it. It also opens you up to accidentally shadowing variables, which can be problematic at best.

5

Since this is the first time I have done this, I am a little unclear on the process. I do understand that you only need to parameterize those items where the values are not static.

If I get $mail populated at run time to be used in my internal function, does it then look something like this?

[pre]

function Create-LocalGroups{

[Cmdletbinding()]
param(
[Parameter(Mandatory = $true)]
[string]$Mail
)

#Internal Function

function New-ForwardGroup
{
param(
[string]$Email = $Mail
)

New-UnifiedGroup -PrimarySMTPAddress $Email

}
}
[/pre]

c:&gt;Create-LocalGroups -mail jane.hancock@contoso.com

6

Yep, that looks perfect! :slight_smile: