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

PowerShell Active Directory Browser

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

Problem

I've just published ADExploder. It's a single-script PowerShell app (source) that uses classic Windows Forms and the Directory Services API to be able to browse nodes and properties of Active Directory servers.

I am interested in hearing feedback about my use of PowerShell, whether it can be made more terse or idiomatic, ways that I can improve my usage of either the Windows Forms or Active Directory API, and any other improvements you can think of.

The full body of the script is here:

```
# Greg Toombs, November 2013

add-type -a (
'System.DirectoryServices',
'System.DirectoryServices.AccountManagement',
'System.Drawing',
'System.Windows.Forms'
)

function GetConnDlg() {
$label = new-object Windows.Forms.Label -pr @{
AutoSize = $true
Dock = [Windows.Forms.DockStyle]::Top
Text = 'Hostname'
}
$text = new-object Windows.Forms.TextBox -pr @{
Anchor = [Windows.Forms.AnchorStyles] 'Left, Right, Top'
MinimumSize = new-object Drawing.Size -a 120, 20
}
$ok = new-object Windows.Forms.Button -pr @{
DialogResult = [Windows.Forms.DialogResult]::OK
Text = '&Connect'
}
$cancel = new-object Windows.Forms.Button -pr @{
DialogResult = [Windows.Forms.DialogResult]::Cancel
Text = 'C&ancel'
}
$flowh = new-object Windows.Forms.FlowLayoutPanel -pr @{
AutoSize = $true
Dock = [Windows.Forms.DockStyle]::Bottom
FlowDirection = [Windows.Forms.FlowDirection]::RightToLeft
}
$flowh.Controls.AddRange(($cancel, $ok))
$flowv = new-object Windows.Forms.FlowLayoutPanel -pr @{
AutoSize = $true
Dock = [Windows.Forms.DockStyle]::Fill
FlowDirection = [Windows.Forms.FlowDirection]::TopDown
}
$flowv.Controls.AddRange(($label, $text))
$conndlg = new-object Windows.Forms.Form -pr @{
AcceptButton = $ok
CancelButton = $cancel
MaximizeBox = $false
MinimizeBox = $false
Padding

Solution

Your code looks pretty good to me.

You can replace this:

[string]::Format('{0} = 0x{0:X8}', $val)


with this:

'{0} = 0x{0:X8}' -f $val


Empty "catch all" catch blocks are not a good idea:

try {
       # ...
    } catch { }


They will hide all exception types indiscriminately, including ones that you might want to know about, which is not a good thing.

If you want to ignore a particular exception type, you should catch just that type (and ideally there should be comment saying why you are ignoring it):

try {
       # ...
    } catch [SomeExceptionType] { 
       # Ignored because blah.
    }


This is a good thing to put at the top of scripts IMO:

Set-StrictMode -Version Latest


It will catch various programming mistakes that might otherwise pass unnoticed.

Code Snippets

[string]::Format('{0} = 0x{0:X8}', $val)
'{0} = 0x{0:X8}' -f $val
try {
       # ...
    } catch { }
try {
       # ...
    } catch [SomeExceptionType] { 
       # Ignored because blah.
    }
Set-StrictMode -Version Latest

Context

StackExchange Code Review Q#55056, answer score: 5

Revisions (0)

No revisions yet.