PSnewbie PSnewbie - 2 months ago 7
PowerShell Question

Powershell finding remote logged on user

I'm fairly new to Powershell and wrote this form for some colleagues. I just wanted to get some advice and guidance on how it's written, what could be better or if it's fine the way it is. Thanks.

enter image description here

[void] [System.Reflection.Assembly]::LoadWithPartialName("System.Windows.Forms")
[void] [System.Reflection.Assembly]::LoadWithPartialName("System.Drawing")

function _Search {
if ($UernameSearch) {Clear-Variable -Name UernameSearch}
$TextSearch = $SearchBox1.Text
$UernameSearch = gwmi win32_computersystem -comp $TextSearch | select USername
if ($UernameSearch) {
if ($UernameSearch.USername) {$OutputBox1.Text = $UernameSearch.USername}
else {$OutputBox1.Text = "No user currently logged on."}}
else {$OutputBox1.Text = "Is $TextSearch offline?"}
$OutputBox1.Enabled = $true

$objForm = New-Object System.Windows.Forms.Form
$objForm.Text = "Who's Logged In"
$Icon = [system.drawing.icon]::ExtractAssociatedIcon($PSHOME + "\powershell.exe")
$objForm.Icon = $Icon
$objForm.Size = New-Object System.Drawing.Size(250,200)
$objForm.StartPosition = "CenterScreen"
$objForm.MaximizeBox = $false
$objForm.FormBorderStyle = 'Fixed3D'
$objForm.KeyPreview = $True
$objForm.Add_KeyDown({if ($_.KeyCode -eq "Escape") {$objForm.Close()}})

$objLabel = New-Object System.Windows.Forms.Label
$objLabel.Location = New-Object System.Drawing.Size(30,5)
$objLabel.AutoSize = $true
$objLabel.TextAlign = "TopCenter"
$objLabel.Text = "Use this tool to find who is currently
logged onto a remote machine."

$objLabe2 = New-Object System.Windows.Forms.Label
$objLabe2.Location = New-Object System.Drawing.Size(60,50)
$objLabe2.Size = New-Object System.Drawing.Size(150,15)
$objLabe2.Text = "Enter Computer Name"

$SearchBox1 = New-Object System.Windows.Forms.TextBox
$SearchBox1.Location = New-Object System.Drawing.Size(15,70)
$SearchBox1.Height = 25
$SearchBox1.Width = 210
$SearchBox1.Multiline = $false
$SearchBox1.Add_KeyDown({if ($_.KeyCode -eq "Enter") {_Search}})

$SearchButton = New-Object System.Windows.Forms.Button
$SearchButton.Location = New-Object System.Drawing.Size(15,95)
$SearchButton.Height = 25
$SearchButton.Width = 210
$SearchButton.Text = "GO!"

$OutputBox1 = New-Object System.Windows.Forms.TextBox
$OutputBox1.Location = New-Object System.Drawing.Size(15,125)
$OutputBox1.Multiline = $false
$OutputBox1.Height = 25
$OutputBox1.Width = 210
$OutputBox1.Multiline = $false
$OutputBox1.Enabled = $false

[void] $objForm.ShowDialog()


Windows.Forms vs WPF

Windows Presentation Foundation is the more advanced alternative. It's nice, but there's a learning curve.

A lot of the display code you have would vanish off into a XAML file (a blob of XML) meaning you can get on with the wiring behind.

Variable naming

Hungarian notation (the "obj" prefix) is a bit pointless. Especially considering that you don't apply it consistently.

Pascal case or camel case are popular conventions these days. e.g.

  • Pascal: MyVariable
  • Camel: myVariable

I have my own, most people do. Pick something you're comfortable with and use it consistently.

function _Search

It's not easy to discover nor is it reusable. Why the underscore?

Consider making a generalised function that returns this information as a collection of objects. Your GUI can consume the output from that function and display it.


Obsolete. It's ambiguous at best. Besides, there are neater PowerShell alternatives:

Add-Type -Assembly System.Windows.Forms


gwmi, select, truncated parameter names: None of these have a place in production code.

Aliaes saves you a few keystrokes and that's fine when you're tapping away at the console. When you're developing something to share they increase obscurity.

Anyone reviewing or updating your code in the future must be familiar with both the command, which at least is descriptive, and the alias.

Consistent formatting

Style is always personal, but there are a small number of very popular ways of writing branching statements like this snippet:

if ($UernameSearch) {
    if ($UernameSearch.USername) {$OutputBox1.Text = $UernameSearch.USername}
    else {$OutputBox1.Text = "No user currently logged on."}}
else {$OutputBox1.Text = "Is $TextSearch offline?"}

Consider the style I prefer:

if ($UsernameSearch) {
    if ($UsernameSearch.Username) {
        $OutputBox1.Text = $UsernameSearch.Username
    } else {
        $OutputBox1.Text = "No user currently logged on."
} else {
    $OutputBox1.Text = "Is $TextSearch offline?"