CODEWITHSUNDEEP
powershellactive-directory
AB_MS3
AB_MS3

Reputation: 77

Which logic to use for loop. Easiest vs most efficient

As I am not an expert programmer, which logic should I use for easy to understand, but efficient script? I am writing a script that searches all AD users and based on an AD attribute(ie office), it'll write some strings to few other attributes (city, postal code, etc). I've been playing around with Foreach and then within that loop using either set-ADUser with replace or where clause. Or the very messy way of a bunch of IF, IfElse statements. Also, I started looking at Switch (Case) statements.

Any suggestions?

Starting small to build upon it, I started with this:

 $Users = Get-ADUser -searchbase "OU=users,OU=testing,DC=blah,DC=company,DC=com" -filter {samaccountname -like "r*"} -properties * | Select samaccountname,streetaddress,l, postalcode,st,physicaldeliveryofficename
foreach ($user in $users) {
set-aduser $user -city "NoWhere" -Postalcode "B2B 2B2"| where {$_.physicaldeliveryofficename -eq "Head Office"} 
}

Or a switch instead:

$Users = Get-ADUser -searchbase "OU=users,OU=testing,DC=blah,DC=company,DC=com" -filter {samaccountname -like "remot*"} -properties * | Select samaccountname,streetaddress,l, postalcode,st,physicaldeliveryofficename
foreach ($user in $users) {switch ($users.physicaldeliveryofficename){
"Nowhere town" { -city "NoWhere" -postalcode "A1A A1A"}
"Anywhereville" { -city "Anywhere" -postalcode "B1B B1B"}
}}

Upvotes: 0

Views: 53

Answers (1)

woxxom
woxxom

Reputation: 73846

  1. Optimize the query because querying complex external resources usually takes 99% of runtime.
  2. Use pipelining so that processing starts immediately without accumulating the entire list:
    instead of ForEach statement use | ForEach { ......... }.
  3. Stop writing oneliners: you can start a new line after |, (, {, ,, unfinished two-operand operators like + or use a single backtick ` at the end of line to wrap long lists of parameters.
  4. Switch is the way to go here because when pipelining there's no entire list available so each element should be processed individually.

Get-ADUser -searchbase "OU=users,OU=testing,DC=blah,DC=company,DC=com" `
           -filter 'samaccountname -like "r*"' `
           -properties * |
    Select samaccountname,
           streetaddress,
           l,
           postalcode,
           st,
           physicaldeliveryofficename |
    ForEach {
        switch ($_.physicaldeliveryofficename) {
            "Nowhere town"  { $city = "NoWhere"; $postalcode = "A1A A1A"; break }
            "Anywhereville" { $city = "Anywhere"; $postalcode = "B1B B1B"; break }
            default         { $city = "?"; $postalcode = "" }
        }
        Set-ADUser $_ -City $city -Postalcode $postalcode
    }

Upvotes: 2

Related Questions