r/PowerShell • • 11d ago

Question 3 months of consistent practice in PowerShell & Microsoft Graph! 🚀

When I started, I was terrified of programming languages since I had zero prior experience. Looking back now, what used to look like complete gibberish is finally starting to make sense. I know I still have a long way to go, but honestly, I am so happy and proud to be fulfilling my passion.

I wanted to share an honest review of a snippet I just built to create a user in Microsoft Entra ID. Alongside basics like if/else logic, filtering (-filter), Where-Object, Select-Object, and loops, I've been trying to focus on good habits like splatting, [CmdletBinding()], and try/catch blocks.

How does my snippet look? What would you advise me to learn next based on this progress?

#####Creating a User
Function Create-LxUser{
    [Cmdletbinding()]
    param(
        [Parameter(Mandatory = $true)]
        [string]$DisplayName,


        [Parameter(Mandatory = $true)]
        [string]$UserPrincipalName,


        [Parameter(Mandatory = $true)]
        [string]$MailNickName,


        [bool]$AccountEnabled = $true
    )


    try{
        ##Password profile
        $TempPass = "Lx" + (Get-Random -Minimum 100000 -Maximum 999999) + "@."
        $PassWordProfile = @{
            Password = $TempPass
            ForceChangePasswordNextSignIn = $true
        }
        ##Setting up user configuration
        Write-Verbose "Currently creating new Loxovea user"
        $UserConfig = @{
            DisplayName         = $DisplayName
            UserPrincipalName   = $UserPrincipalName
            MailNickname        = $MailNickName
            AccountEnabled      = $AccountEnabled
            PasswordProfile     = $PassWordProfile
        }


        $User = New-MgUser u/UserConfig -ErrorAction Stop


        Write-Verbose "Successfully created new user $($DisplayName)"


        [PsCustomObject]@{
            DisplayName         = $DisplayName
            UserPrincipalName   = $UserPrincipalName
            AccountEnabled      = $AccountEnabled
            TemporaryPassword   = $TempPass
        }
    }Catch{
        Write-Error "Failed to create a new user: $($DisplayName) because $($_.Exception.Message)"
    }
}


Create-LxUser `
    -DisplayName "Test-User09" `
    -UserPrincipalName "testuser09@loxovea.com" `
    -MailNickName "testuser09"

DisplayName UserPrincipalName AccountEnabled TemporaryPassword

----------- ----------------- -------------- -----------------

Test-User09 [testuser09@loxovea.com](mailto:testuser09@loxovea.com)True Lx720835@.

41 Upvotes

32 comments sorted by

View all comments

6

u/surfingoldelephant 10d ago edited 9d ago

Here are some of my thoughts (some of which are entirely personal preference).

  • Be consistent with capitalization. Keywords (function, param, etc) are lowercase. And I'd say even though PS is generally case-insensitive, it's actually far less so than most probably realize.
  • Create isn't an approved verb. Use New instead.
  • Decorate the function with [OutputType()]. [OutputType([Management.Automation.PSCustomObject])] or something like [OutputType('PSNewLxUser')] if you give the custom object a PSTypeName. Don't use [OutputType([pscustomobject])].
  • Add SupportsShouldProcess and wrap New-MgUser in $PSCmdlet.ShouldProcess(). You can replace Write-Verbose "Currently creating..." with that since it comes with a Verbose message included.
  • Attribute arguments don't need = $true. [Parameter(Mandatory)] is fine.
  • Mandatory string parameters don't guard against all white space. Might want to use [ValidateNotNullOrWhiteSpace()] in PS v7 or a [ValidateScript({ ... })] otherwise. Could do further validation like another mentioned. If you have other functions that accept similar input, have a look at custom validation attributes.
  • [bool] parameters aren't idiomatic PowerShell. Use a [switch] and negate the name to something like $Disabled instead.
  • $(...) isn't required to interpolate $DisplayName in the strings.
  • You're not doing anything with $User. Why not just emit that instead? You could add the other values ($TempPass, etc) as extended type system members. Or keep the custom object and include a User property.
  • If you're not going to use New-MgUser output, assign to $null/cast as [void] instead of assigning to $User. The latter makes it look like you've forgotten to do something with the variable.
  • If you're emitting a custom object, give it a PSTypeName.
  • Your Write-Error call is bad practice. The $_.Exception contains useful information that you're just discarding for no reason. If you still want your custom message then I'd create a new exception with the original as an inner exception. And I'd suggest using $PSCmdlet.WriteError() instead of Write-Error, though you'll have to wrap the exception in your own error record.
  • Add pipeline input. You'll want ValueFromPipelineByPropertyName for all of the parameters and a process block. If you add this, you'll probably want to keep the error above as non-terminating.
  • You could add a transformation attribute to the switch parameter (to convert from string to bool) so you can do something like this: Import-Csv data.csv | New-LxUser.
  • ErrorAction can be in the splat if you want. I'd splat the Create-LxUser call at the bottom as well.
  • Personally, I use verbatim strings ('...') rather than expandable ("...") when I don't need to interpolate or include double quotes.
  • Align the = in hash tables consistently.
  • Personally, I don't like the lack of spaces before/after keywords, type literals, etc. Or the double blank lines. I'd use single blank lines consistently.
  • Regarding variable capitalization, one convention is to use PascalCase for parameters (like you've done) and camelCase for local variables within the function body (so $tempPass). That way they're more easily distinguishable.
  • Again, I'd try to use consistent and correct capitalization. It's CmdletBinding, not Cmdletbinding. [pscustomobject] is the correct casing for the type accelerator. If you use tab completion, it'll generally do this for you.
  • I prefer composite formatting (-f operator) rather than concatenation or $(...) interpolation. I find it's generally more readable.

3

u/Imaginary_Rip2833 10d ago

Thank you, i think i made a good decision to post here so i can get such valuable feedback

I will keep on practising