patternshellMinor
Getting a list of all computer objects in each AD domain
Viewed 0 times
eachobjectsallgettingcomputerlistdomain
Problem
So I wrote this script and although it does work I can't help but think that it can be more efficient with all the repetitive code. Can anyone offer any suggestions on making it fewer lines and eliminating the repeat lines?
```
$Domains = (Get-ADForest).domains | %{Get-ADDomain $_ | Select DistinguishedName,ReplicaDirectoryServers}
#Gets list of all Computer objects in each domain
foreach ($Domain in $Domains){
Write-Output "Working on $($Domain.DistinguishedName)"
#Iterates through Computer array
foreach ($Comp in (Get-ADComputer -Server $($Domain.ReplicaDirectoryServers[0]) -SearchBase $($Domain.DistinguishedName) -filter {OperatingSystem -notlike "Server"} -Properties Name,LastLogonDate,DistinguishedName| ?{ $_.DistinguishedName -notlike "OU=Disabled" -and $_.DistinguishedName -notlike "OU=Servers"}|Select Name,LastLogonDate,DistinguishedName)){
#Performs test on date, if date is greater then $Days moves to $OU.
if(((New-TimeSpan -Start ($Comp.LastLogonDate) -End (Get-Date)).days) -gt $Days -and $Exclusion -notcontains $Comp.name){
$Computers += $Comp
#Finds what OU each object should go to. Checks if object is already in dumpster OU
if($($domain.DistinguishedName) -like "DomainA"){
Write-Output ((Get-Date).ToString('MM-dd-yyyy:hh:mm:ss') + ": Moved $($comp.DistinguishedName) to $DomainAOUDump") | Out-File $log -Append
Move-ADObject -Identity $comp.DistinguishedName -Server $($Domain.ReplicaDirectoryServers[0]) -TargetPath $DomainAOUDump
}
elseif($($domain.DistinguishedName) -like "DomainB"){
Write-Output ((Get-Date).ToString('MM-dd-yyyy:hh:mm:ss') + ": Moved $($comp.DistinguishedName) to $DomainBOUDump") | Out-File $log -Append
Move-ADObject -Identity $comp.DistinguishedName -Server $($Domain.ReplicaDirectoryServers[0]) -TargetPath $DomainBOUDump
}
elseif($($domain.DistinguishedName) -like "DomainC"){
Write-Output
```
$Domains = (Get-ADForest).domains | %{Get-ADDomain $_ | Select DistinguishedName,ReplicaDirectoryServers}
#Gets list of all Computer objects in each domain
foreach ($Domain in $Domains){
Write-Output "Working on $($Domain.DistinguishedName)"
#Iterates through Computer array
foreach ($Comp in (Get-ADComputer -Server $($Domain.ReplicaDirectoryServers[0]) -SearchBase $($Domain.DistinguishedName) -filter {OperatingSystem -notlike "Server"} -Properties Name,LastLogonDate,DistinguishedName| ?{ $_.DistinguishedName -notlike "OU=Disabled" -and $_.DistinguishedName -notlike "OU=Servers"}|Select Name,LastLogonDate,DistinguishedName)){
#Performs test on date, if date is greater then $Days moves to $OU.
if(((New-TimeSpan -Start ($Comp.LastLogonDate) -End (Get-Date)).days) -gt $Days -and $Exclusion -notcontains $Comp.name){
$Computers += $Comp
#Finds what OU each object should go to. Checks if object is already in dumpster OU
if($($domain.DistinguishedName) -like "DomainA"){
Write-Output ((Get-Date).ToString('MM-dd-yyyy:hh:mm:ss') + ": Moved $($comp.DistinguishedName) to $DomainAOUDump") | Out-File $log -Append
Move-ADObject -Identity $comp.DistinguishedName -Server $($Domain.ReplicaDirectoryServers[0]) -TargetPath $DomainAOUDump
}
elseif($($domain.DistinguishedName) -like "DomainB"){
Write-Output ((Get-Date).ToString('MM-dd-yyyy:hh:mm:ss') + ": Moved $($comp.DistinguishedName) to $DomainBOUDump") | Out-File $log -Append
Move-ADObject -Identity $comp.DistinguishedName -Server $($Domain.ReplicaDirectoryServers[0]) -TargetPath $DomainBOUDump
}
elseif($($domain.DistinguishedName) -like "DomainC"){
Write-Output
Solution
One thing that I notice, you can change this repetitive fragment
Into the following (minus some variable definitions, e.g.
with
So, the script will become (minus definition for necessary functions to hide the complexity)
Note that the
For further cleaning, you can redefine how you store the
Note: I'm not familiar with ADObject and stuff, so the variable naming might be a little off.
if($($domain.DistinguishedName) -like "*DomainA*"){
Write-Output ((Get-Date).ToString('MM-dd-yyyy:hh:mm:ss') + ": Moved $($comp.DistinguishedName) to $DomainAOUDump") | Out-File $log -Append
Move-ADObject -Identity $comp.DistinguishedName -Server $($Domain.ReplicaDirectoryServers[0]) -TargetPath $DomainAOUDump
}Into the following (minus some variable definitions, e.g.
$TimeStamp):$DomainPrefix = Get-DomainPrefix $Domain
$TargetPath = Get-Variable -Name "Domain$($DomainPrefix)OUDump"
Write-Output "$TimeStamp: Moved $DistinguishedName to $TargetPath" | Out-File $log -Append
Move-ADObject -Identity $DistinguishedName -Server $Server -TargetPath $TargetPathwith
Get-DomainPrefix defined to capture the first character after "Domain", e.g.Function Get-DomainPrefix([String] $DomainName){
$DomainName -match "Domain(?.)" | Out-Null
$Matches.Prefix
}So, the script will become (minus definition for necessary functions to hide the complexity)
foreach ($Domain in Get-Domains) {
foreach ($Comp in (Get-ComputerArray -Domain $Domain)) {
if ( ShouldBeMoved -Computer $Comp ) {
$DomainPrefix = Get-DomainPrefix $Domain
$TargetPath = Get-Variable -Name "Domain$($DomainPrefix)OUDump"
$TimeStamp = Get-TimeStamp
$CompName = $Comp.DistinguishedName
$Server = Get-Server
Write-Output "$TimeStamp: Moved $CompName to $TargetPath" | Out-File $log -Append
Move-ADObject -Identity $CompName -Server $Server -TargetPath $TargetPath
}
}
}Note that the
else clause is not handled in the simplified script above.For further cleaning, you can redefine how you store the
$TargetPath into a more friendly structure so that we don't need to use Get-Variable. Perhaps a map from DomainPrefix to the TargetPath.Note: I'm not familiar with ADObject and stuff, so the variable naming might be a little off.
Code Snippets
if($($domain.DistinguishedName) -like "*DomainA*"){
Write-Output ((Get-Date).ToString('MM-dd-yyyy:hh:mm:ss') + ": Moved $($comp.DistinguishedName) to $DomainAOUDump") | Out-File $log -Append
Move-ADObject -Identity $comp.DistinguishedName -Server $($Domain.ReplicaDirectoryServers[0]) -TargetPath $DomainAOUDump
}$DomainPrefix = Get-DomainPrefix $Domain
$TargetPath = Get-Variable -Name "Domain$($DomainPrefix)OUDump"
Write-Output "$TimeStamp: Moved $DistinguishedName to $TargetPath" | Out-File $log -Append
Move-ADObject -Identity $DistinguishedName -Server $Server -TargetPath $TargetPathFunction Get-DomainPrefix([String] $DomainName){
$DomainName -match "Domain(?<Prefix>.)" | Out-Null
$Matches.Prefix
}foreach ($Domain in Get-Domains) {
foreach ($Comp in (Get-ComputerArray -Domain $Domain)) {
if ( ShouldBeMoved -Computer $Comp ) {
$DomainPrefix = Get-DomainPrefix $Domain
$TargetPath = Get-Variable -Name "Domain$($DomainPrefix)OUDump"
$TimeStamp = Get-TimeStamp
$CompName = $Comp.DistinguishedName
$Server = Get-Server
Write-Output "$TimeStamp: Moved $CompName to $TargetPath" | Out-File $log -Append
Move-ADObject -Identity $CompName -Server $Server -TargetPath $TargetPath
}
}
}Context
StackExchange Code Review Q#90199, answer score: 6
Revisions (0)
No revisions yet.