Solved

Revise PowerShell Script

Posted on 2013-01-24
3
284 Views
Last Modified: 2013-01-25
I have a script (that came from a previous question). This script creates new computers off of an existing computer template.

As I am still working with PowerShell, I would like to see how it can be approved in efficiency or syntax. The script is currently working but I am sure it can be better.

If it can be improved, can you also explain why an improvement should be made?

Add-PSSnapin Quest.ActiveRoles.ADManagement


$SourceComputer=Read-Host "What will be the computer template?"
$Computer=Get-QADComputer $SourceComputer
$ParentContainer=$Computer.ParentContainer
$Groups=Get-QADComputer $SourceComputer | Get-QADMemberof


$prefix = Read-Host "What is the computer prefix? EX: GAMCN or GAMCLABN"
try { [int]$startnumber = Read-Host "What number would you like to start with? EX: 01 or 13" -ea "Stop"}
catch {"Need to enter a number"; break}
try { [int]$number = Read-Host "How many computer accounts do you want to create?" -ea "Stop"}
catch {"Need to enter a number"; break}
ForEach ($i in ($startnumber..($startnumber + $number -1)))
{
  $compName = $prefix + $i
  New-QADComputer -name $compName -ParentContainer $ParentContainer
  
  foreach ($Group in $Groups) {
  Add-QADGroupMember $Group $compName}

} 

Open in new window

0
Comment
Question by:Joseph Moody
3 Comments
 
LVL 21

Expert Comment

by:mcsween
ID: 38816014
This code looks pretty good to me; I can't really see any improvements to be made.  In VBScript this would be twice as long.
0
 
LVL 69

Accepted Solution

by:
Qlemo earned 500 total points
ID: 38816680
The only improvement I can think of is to use less variables. Whenever possible, intermediate objects should not be kept in vars, to allow for lean memory usage. It doesn't matter here, as the vars are "static", but as a rule of thumb, as soon as you are processing a bunch of objects. Your lines 4 to 7 would then look like this:
$SourceComputer = Read-Host "What will be the computer template?"
$ParentContainer = (Get-QADComputer $SourceComputer).ParentContainer
$Groups = Get-QADComputer $SourceComputer | Get-QADMemberof

Open in new window

I don't know if there is any effect, but it might perform better if you collect the generated PC names, and stuff them into a single Add-QADGroupMember call for each group. Starting from line 15:
$comps = @()
ForEach ($i in ($startnumber..($startnumber + $number -1)))
{
  $comps += ($compName = $prefix + $i)
  New-QADComputer -name $compName -ParentContainer $ParentContainer
}

foreach ($Group in $Groups) {
  Add-QADGroupMember $Group $comps
}

Open in new window

0
 
LVL 22

Author Closing Comment

by:Joseph Moody
ID: 38818390
Thank you very much for the feedback!

I had thought that lines 4-7 could be condensed but couldn't figure out how.
0

Featured Post

NAS Cloud Backup Strategies

This article explains backup scenarios when using network storage. We review the so-called “3-2-1 strategy” and summarize the methods you can use to send NAS data to the cloud

Question has a verified solution.

If you are experiencing a similar issue, please ask a related question

Suggested Solutions

Active Directory replication delay is the cause to many problems.  Here is a super easy script to force Active Directory replication to all sites with by using an elevated PowerShell command prompt, and a tool to verify your changes.
This article explains how to prepare an HTML email signature template file containing dynamic placeholders for users' Azure AD data. Furthermore, it explains how to use this file to remotely set up a department-wide email signature policy in Office …
Two types of users will appreciate AOMEI Backupper Pro: 1 - Those with PCIe drives (and haven't found cloning software that works on them). 2 - Those who want a fast clone of their boot drive (no re-boots needed) and it can clone your drive wh…

828 members asked questions and received personalized solutions in the past 7 days.

Join the community of 500,000 technology professionals and ask your questions.

Join & Ask a Question