Discover-MSSQLServers suggestions

[mattifestation]

• Rename 'function Discover-MSSQLServers.ps1' to 'function Discover-MSSQLServers'
• Remove the GroupResults and SortResults functionality. You should only output object and let the user figure out what to do with the raw output.
• Is there a distinct performance hit when -ExtendedInfo is provided? If not, remove the -ExtendedInfo switch and just query those properties by default. Ideally, a function shouldn't output more than one type of object.
• Only output objects! The following line is the equivalent of Write-Host and is unnecessary: Write-Output "Discovered $AllMSSQLServerFQDNsCount servers running MS SQL `r "

• When creating custom object, creating a hastable of properties and adding them to a PSObject seems to be the unofficial best practice. e.g. $ObjectProperties = @{ Prop1 = 1 Prop2 = 2 }

  New-Object -TypeName PSObject -Property $ObjectProperties

Otherwise, everything looks good! Now, I don't have a domain environment in which I can test this but perhaps @obsuresec does. Nice work!

-Matt

[PyroTek3]

Thanks for the feedback! I am not a developer, more of a PowerShell enthusiast & hacker (to use the original definition) and , obviously, not used to writing complete scripts and not just functions. ☺

Good call on the write-output. That was more of a testing artifact than anything else. I will remove it in the next update (along with the sorting/grouping options).

I have a pretty substantial lab at home which I tested against while coding the scripts and will run against a couple of customer environments this week to determine true performance (one is a single domain, single forest and another is a multiple domain, single forest). Comparing run-time with ExtendedInfo and without will help determine if it’s significant.

I’ll get the other scripts updated in similar fashion to the comments you provided on this one as I’m sure they’re applicable. I look forward to your (and Chris’) feedback on the other scripts as well (the ADInfo one is still a work in progress – still determining desired output on that one).

From: Matt Graeber [mailto:notifications@github.com] Sent: Monday, September 1, 2014 6:16 PM To: PyroTek3/PowerShell Subject: [PowerShell] Discover-MSSQLServers suggestions (#1)

• Rename 'function Discover-MSSQLServers.ps1' to 'function Discover-MSSQLServers'
• Remove the GroupResults and SortResults functionality. You should only output object and let the user figure out what to do with the raw output.
• Is there a distinct performance hit when -ExtendedInfo is provided? If not, remove the -ExtendedInfo switch and just query those properties by default. Ideally, a function shouldn't output more than one type of object.
• Only output objects! The following line is the equivalent of Write-Host and is unnecessary: Write-Output "Discovered $AllMSSQLServerFQDNsCount servers running MS SQL `r "

· When creating custom object, creating a hastable of properties and adding them to a PSObject seems to be the unofficial best practice. e.g. $ObjectProperties = @{ Prop1 = 1 Prop2 = 2 }

New-Object -TypeName PSObject -Property $ObjectProperties

Otherwise, everything looks good! Now, I don't have a domain environment in which I can test this but perhaps @obsuresec does. Nice work!

-Matt

— Reply to this email directly or view it on GitHubhttps://github.com/PyroTek3/PowerShell/issues/1.

[PyroTek3]

Replying to twitter DM on email for content reasons.

"Awesome! I still think you should use a custom object though instead of outputting an array of objects in $ALLSQLServerReport :)"

I thought I had used a custom object as per

http://technet.microsoft.com/en-us/library/ff730946.aspx (second link in the PowerSploit style guide)

"

$colAverages = @() $colStats = Import-CSV C:\Scripts\Test.txt

foreach ($objBatter in $colStats) { $objAverage = New-Object System.Object $objAverage | Add-Member -type NoteProperty -name Name -value $objBatter.Name $objAverage | Add-Member -type NoteProperty -name BattingAverage -value ("{0:N3}" -f ([int] $objBatter.Hits / $objBatter.AtBats)) $colAverages += $objAverage } $colAverages | Sort-Object BattingAverage -descending "

I did restructure it as you suggested below, but couldn't figure out the proper looping to merge the data in an object, so I altered it to better match the above.

I am always open to better methods. :)

Due to some testing today, I am dumping "ExtendedInfo" for now.

I am also changing the processing for the Interesting Services.

Thanks for the feedback. This project is helping me expand my ADSI knowledge as well as PowerShell optimization.

Cheers,

Sean

sean@metcorp.com (personal)

sean@dansolutions.com (work)

---

From: Sean Metcalf Sent: Monday, September 01, 2014 9:35 PM To: PyroTek3/PowerShell Subject: RE: [PowerShell] Discover-MSSQLServers suggestions (#1)

Thanks for the feedback! I am not a developer, more of a PowerShell enthusiast & hacker (to use the original definition) and , obviously, not used to writing complete scripts and not just functions. :)

Good call on the write-output. That was more of a testing artifact than anything else. I will remove it in the next update (along with the sorting/grouping options).

I have a pretty substantial lab at home which I tested against while coding the scripts and will run against a couple of customer environments this week to determine true performance (one is a single domain, single forest and another is a multiple domain, single forest). Comparing run-time with ExtendedInfo and without will help determine if it's significant.

I'll get the other scripts updated in similar fashion to the comments you provided on this one as I'm sure they're applicable. I look forward to your (and Chris') feedback on the other scripts as well (the ADInfo one is still a work in progress - still determining desired output on that one).

From: Matt Graeber [mailto:notifications@github.com] Sent: Monday, September 1, 2014 6:16 PM To: PyroTek3/PowerShell Subject: [PowerShell] Discover-MSSQLServers suggestions (#1)

• Rename 'function Discover-MSSQLServers.ps1' to 'function Discover-MSSQLServers'
• Remove the GroupResults and SortResults functionality. You should only output object and let the user figure out what to do with the raw output.
• Is there a distinct performance hit when -ExtendedInfo is provided? If not, remove the -ExtendedInfo switch and just query those properties by default. Ideally, a function shouldn't output more than one type of object.
• Only output objects! The following line is the equivalent of Write-Host and is unnecessary: Write-Output "Discovered $AllMSSQLServerFQDNsCount servers running MS SQL `r "

· When creating custom object, creating a hastable of properties and adding them to a PSObject seems to be the unofficial best practice. e.g. $ObjectProperties = @{ Prop1 = 1 Prop2 = 2 }

New-Object -TypeName PSObject -Property $ObjectProperties

Otherwise, everything looks good! Now, I don't have a domain environment in which I can test this but perhaps @obsuresec does. Nice work!

-Matt

Reply to this email directly or view it on GitHubhttps://github.com/PyroTek3/PowerShell/issues/1.