HiveBrain v1.2.0
Get Started
← Back to all entries
patternMinor

VBScript for AD account creation

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
accountcreationvbscriptfor

Problem

Does the following code look okay? Want to see if anyone can spot any mistakes I may have made. This will be pulling from a pre-formatted Excel document - I have already specified all needed Cell locations.

Things I want clarified:

-
How many times can you invoke SetInfo and did I use it more than needed? I was under the impression that a SetInfo would be needed before a SetPassword can be used, but I could be very wrong.

-
Does this line look okay? Was I correct to separate multiple cells using a comma in order to put all the info together? I know it worked with an Echo command:

Set objUser = objOU.Create _
("User", "cn=" & objExcel.Cells(intRow, 3).Value, objExcel.Cells(intRow, 2).Value, objExcel.Cells(intRow, 19).Value)


-
Is this the correct code for setting a password to never expire?

objUser.Put "userAccountControl", intUAC XOR _
ADS_UF_DONT_EXPIRE_PASSWD


-
Full code below, I'd appreciate any feedback on anything else that is found.

```
Set objExcel = CreateObject("Excel.Application")
Set objWorkbook = objExcel.Workbooks.Open _
("C:\Book1.xls")
intRow = 2
Do Until objExcel.Cells(intRow,1).Value = ""
Set objOU = GetObject _
("ou=" & objExcel.Cells(intRow, 13).Value & _
", dc=satdc, dc=com")
Set objUser = objOU.Create _
("User", "cn=" & objExcel.Cells(intRow, 3).Value, objExcel.Cells(intRow, 2).Value, objExcel.Cells(intRow, 19).Value)
objUser.sAMAccountName = objExcel.Cells(intRow, 19).Value & ".sat"
objUser.GivenName = objExcel.Cells(intRow, 3).Value
objUser.SN = objExcel.Cells(intRow, 2).Value
objUser.SetInfo
objUser.AccountDisabled = False
objUser.AccountExpirationDate = Date + 365
objUser.SetPassword objExcel.Cells(intRow, 9).Value
objUser.SetInfo
objUser.Put "userAccountControl", intUAC XOR _
ADS_UF_DONT_EXPIRE_PASSWD
objUser.HomeDirectory = "\\satdc" & "\" & "Users" & "\" & _
objUser.Get("sAMAccountName")
objUser.homeDrive = "U:"
objUs

Solution

Naming: Hungarian Notation is the DEVIL.
It makes the reader want to see your code burn in eternal flames.

https://simpsonswiki.com/wiki/Flanders_the_Devil

  • objExcel would be better off as xlApp or excelApp



  • objUser is dying to call itself something like adUser



  • intRow wants to be called xlRow or just row



  • intUAC is silently begging for a more descriptive name



  • objOU wants to shoot whoever named it that



Now, whether your code is correct or not is off-topic for this site; since this question hasn't been closed as off-topic yet, I'm going to assume you've got working code.

Do Until objExcel.Cells(intRow,1).Value = ""


I'd use vbNullString here instead of "". It makes the intent clearer, and the non-string takes up 0 bytes. "" eats 6 bytes for no reason - StrPtr(vbNullString) is 0; StrPtr("") is a non-zero memory address.

The repeated assignments on objUser are a missed opportunity of using a With block:

With objOU.Create("User", "cn=" & objExcel.Cells(intRow, 3).Value, _
                          objExcel.Cells(intRow, 2).Value, _
                          objExcel.Cells(intRow, 19).Value)

    .SAMAccountName = objExcel.Cells(intRow, 19).Value & ".sat"
    .GivenName = objExcel.Cells(intRow, 3).Value
    .SN = objExcel.Cells(intRow, 2).Value
    .AccountDisabled = False
    .AccountExpirationDate = Date + 365
    '...

End With


Your usage of line continuation characters is... well, you use it in weird places. Actually, everywhere you've used it, I wouldn't have. Notice in the above snippet, how I used it to line up the parameters.

Code Snippets

Do Until objExcel.Cells(intRow,1).Value = ""
With objOU.Create("User", "cn=" & objExcel.Cells(intRow, 3).Value, _
                          objExcel.Cells(intRow, 2).Value, _
                          objExcel.Cells(intRow, 19).Value)

    .SAMAccountName = objExcel.Cells(intRow, 19).Value & ".sat"
    .GivenName = objExcel.Cells(intRow, 3).Value
    .SN = objExcel.Cells(intRow, 2).Value
    .AccountDisabled = False
    .AccountExpirationDate = Date + 365
    '...

End With

Context

StackExchange Code Review Q#23583, answer score: 5

Revisions (0)

No revisions yet.