Getting essential computer info for network admins

Question

I recently had to install VNC on mass across a lot of computers on a domain and needed some info about their PC, for example:

1. Host name
2. Username
3. MAC Address
4. IPv4 Address
5. Domain name

Once all of this info is gathered, it is compiled into a text file and uploaded to a FTP server. To save files from being conflicted with the same name, I set each file to be named the name of the user.

This is the first ever script and first experience I have had with Powershell so please tell me if there is anything I can improve.

function Get-MACAddress {
 ipconfig /all | findstr "Physical" | Where-Object {$_.length -lt 58}
}
function Get-IPAddress {
 ipconfig | findstr "IPv4 Address"
}
function Get-HostName {
 get-content env:computername
}
function Get-UserName{
 get-content env:UserName
}
function Get-DomainName{
get-content env:USERDOMAIN
}
function CreateCSVdocument {
 Get-MACAddress | New-Item C:\Users\$($env:username)\Documents\$($env:username).txt -type file -force 
 Add-Content C:\Users\$($env:username)\Documents\$($env:username).txt " "
Get-IPAddress | Add-Content C:\Users\$($env:username)\Documents\$($env:username).txt
Add-Content C:\Users\$($env:username)\Documents\$($env:username).txt " "
Add-Content C:\Users\$($env:username)\Documents\$($env:username).txt "Hostname:"
Get-Hostname | Add-Content C:\Users\$($env:username)\Documents\$($env:username).txt
Add-Content C:\Users\$($env:username)\Documents\$($env:username).txt " "
Add-Content C:\Users\$($env:username)\Documents\$($env:username).txt "User:"
Get-UserName | Add-Content C:\Users\$($env:username)\Documents\$($env:username).txt
Add-Content C:\Users\$($env:username)\Documents\$($env:username).txt " "
Add-Content C:\Users\$($env:username)\Documents\$($env:username).txt "Domain:"
Get-DomainName | Add-Content C:\Users\$($env:username)\Documents\$($env:username).txt
}
function FTPUpload {
$Dir="C:\Users\$($env:username)\Documents\$($env:username).txt" 
#ftp server 
$ftp = "ftpserver" 
$user = "username" 
$pass = "password" 
$webclient = New-Object System.Net.WebClient 
$webclient.Credentials = New-Object System.Net.NetworkCredential($user,$pass) 
#list every sql server trace file 
foreach($item in (dir $Dir "$($env:username).txt")){ 
 "Uploading $item..." 
 $uri = New-Object System.Uri($ftp+$item.Name) 
 $webclient.UploadFile($uri, $item.FullName) 
} 
}
Get-DomainName
Get-IPAddress
Get-MACAddress
Get-HostName
Get-UserName
CreateCSVdocument
FTPUpload

Answer 1

It looks quite fine to me.

I'm mainly surprised by the massive duplication in the CreateCSVdocument function:

function CreateCSVdocument {
 Get-MACAddress | New-Item C:\Users\$($env:username)\Documents\$($env:username).txt -type file -force 
 Add-Content C:\Users\$($env:username)\Documents\$($env:username).txt " "
Get-IPAddress | Add-Content C:\Users\$($env:username)\Documents\$($env:username).txt
Add-Content C:\Users\$($env:username)\Documents\$($env:username).txt " "
Add-Content C:\Users\$($env:username)\Documents\$($env:username).txt "Hostname:"
Get-Hostname | Add-Content C:\Users\$($env:username)\Documents\$($env:username).txt
Add-Content C:\Users\$($env:username)\Documents\$($env:username).txt " "

It would be better to introduce a variable there and use that, for example:

function CreateCSVdocument {
 $Dir="C:\Users\$($env:username)\Documents\$($env:username).txt" 
 Get-MACAddress | New-Item $Dir -type file -force 
 Add-Content $Dir " " 
 Get-IPAddress | Add-Content $Dir
 Add-Content $Dir " "
 Add-Content $Dir "Hostname:"
 Get-Hostname | Add-Content $Dir
 Add-Content $Dir " "

Actually, this is still too repetitive. Probably there is a better way to output all the text and pipe the whole thing to ... | Add-Content $Dir once at the end, but I don't know how to do that in powershell.

---

The formatting is inconsistent:

• In function definitions, sometimes you put a space between function name and { and sometimes you don't:

  function Get-MACAddress {
  function Get-UserName{
  

  I recommend to put a space always, consistently.

• Indentation is inconsistent:

  • You correctly indented the body of Get-MACAddress, Get-IPAddress and others
  • But you didn't follow the same logic in Get-HostName, Get-UserName, Get-DomainName and others

• Phasmatis: Thank's Janos, this has made it look a lot neater. I have put the 'Add-Content $Dir " "' to create a new line as when I have tried other code to create a new line it does not work. could you please advice?
• janos: @Phasmatis I don't see why that wouldn't work.
• Phasmatis: it seems to write it in the text file also, for example if I did /n it would create a new line but write /n in the text document also.

Answer 2

The script looks sound and functional, and quite a useful script considering it's your first. This is how I would restructure it for brevity and readability:

• I would do away with the functions altogether, unless this is part of a larger script where you will call them multiple times.

• The bulk of the script can be condensed using @" ... "@. You can insert new lines to the output, and include the information you need:

  $outputfile = "C:\Users\$($env:username)\Documents\$($env:username).txt"
  @"
  $(ipconfig /all | ? { $_ -ilike "*physical*" -and $_.length -lt 58 })
  $(ipconfig | ? { $_ -ilike "*IPv4 Address*" })
  Hostname: $env:computername
  User: $env:UserName
  Domain: $env:USERDOMAIN
  "@ | Out-File $outputfile
  

• Notice I didn't use findstr - there is functionality in Powershell for this. You touched on it when testing $_.length -lt 58.

• Declaring $Dir would now be unnecessary due to declaring $outputfile on the first line:

  $Dir="C:\Users\$($env:username)\Documents\$($env:username).txt"
  

• I don't have access to an FTP server to test the upload, but this loop is suspicious:

  foreach($item in (dir $Dir "$($env:username).txt")){ ... }
  

  Using dir is fine, but note this is an alias of the Powershell command Get-ChildItem. The loop condition here amounts to getting a single file, whose name and path is already stored in $outputfile on the first line. Therefore the entire loop is unnecessary:

  $item = Get-Item -Path $outputfile
  "Uploading $($item.FullName)..." 
  $uri = New-Object System.Uri($ftp+$item.Name)
  $webclient.UploadFile($uri, $item.FullName)