Getting a list of all computer objects in each AD domain

Question

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 ((Get-Date).ToString('MM-dd-yyyy:hh:mm:ss') + ": Moved $($comp.DistinguishedName) to $DomainCOUDump") | Out-File $log -Append
 Move-ADObject -Identity $comp.DistinguishedName -Server $($Domain.ReplicaDirectoryServers[0]) -TargetPath $DomainCOUDump
 }
 elseif($($domain.DistinguishedName) -like "*DomainD*"){
 Write-Output ((Get-Date).ToString('MM-dd-yyyy:hh:mm:ss') + ": Moved $($comp.DistinguishedName) to $DomainDOUDump") | Out-File $log -Append
 Move-ADObject -Identity $comp.DistinguishedName -Server $($Domain.ReplicaDirectoryServers[0]) -TargetPath $DomainDOUDump
 }
 else{
 Write-Output ((Get-Date).ToString('MM-dd-yyyy:hh:mm:ss') + ": I don't know where $($comp.name) should be moved") | Out-File $log -Append
 }
 }
 }
 }

• mjolinor: I'd start by trading that stack of if/elseif/else statements for a Switch.
• Phrancis: Welcome to Code Review! I hope you get some good answers!

Answer 1

One thing that I notice, you can change this repetitive fragment

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 $TargetPath

with Get-DomainPrefix defined to capture the first character after "Domain", e.g.

Function Get-DomainPrefix([String] $DomainName){
 $DomainName -match "Domain(?<Prefix>.)" | 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.