0

After an Application install we are required to count the GAC .dll file so I created this script that works. However while I think it's OK (for me) I don't have anyone to review it. Are there any improvements you want to recommend?

$path = "C:\Windows\Microsoft.NET\assembly\GAC_MSIL"
$app = Read-Host -Prompt 'Enter BizTalk App Name example (Order)'
Get-ChildItem -Path $path -Recurse -include "*$app*.dll" | Measure-Object | %{$_.Count} | Write-Host

Thanks in advance, I've learned a lot on this site.

Maximilian Burszley
  • 18,243
  • 4
  • 34
  • 63
jcarreiro
  • 39
  • 1
  • 9
  • 3
    This may be better asked on [codereview.se]. – Jeff Zeitlin Mar 19 '18 at 17:48
  • It certainly is a code review question. I will tell you that there is no need to pass to %{$_.Count}. you could either wrap the beginning up until after Measure-Object in parens and access the .Count property or replace the for-eachobject loop with Select-Object -expand Count. Also most people frown on Write-Host except for very specific situations. – EBGreen Mar 19 '18 at 18:11
  • `@(Get-ChildItem -Path $Path -Recurse -Include "*$app*.dll").Count` – Maximilian Burszley Mar 19 '18 at 18:18
  • No need to wrap that in `Write-Host` and `Measure-Object` – Maximilian Burszley Mar 19 '18 at 18:18

1 Answers1

1

It's a 3-line script, it's pretty much fine. But if you want feedback, the two things I'd pick up on are:

  1. Use of % in a script, it's an alias and general PowerShell advice is to avoid using aliases in scripts. Putting the full cmdlet name ForEach-Object makes it clearer to people who haven't memorised the aliases. The PowerShell Community StyleGuide mentions that in the naming conventions page:

  2. Use of Write-Host - it's something to use only when you need it. PowerShell output should be to the pipeline instead of to the host, unless you definitely need host output. It makes your scripts more composable - you can't pipe from one script to another if it uses Write-Host for output.

    There's a lot written about this with more discussion, e.g.

Smaller changes:

  • Add a comment to say what it does or why.
  • "string" - strings in double quotes can expand variables and sub-expressions. Strings in single quotes cannot. Since your path has no variables in it, put it in single quotes and that tells anyone reading the script that they should not look for or expect any variables or subexpressions in the string.
  • -Path $path - the -Path parameter can handle wildcards. Your path is a literal directory name with no wildcards, use -LiteralPath $path as a way to avoid any accidental wildcards (e.g. [] in filenames are treated as wildcards).

Then it would be

# Count the Global Assembly Cache DLL files for a given app name

$path = 'C:\Windows\Microsoft.NET\assembly\GAC_MSIL'
$app = Read-Host -Prompt 'Enter BizTalk App Name example (Order)'

Get-ChildItem -LiteralPath $path -Recurse -Include "*$app*.dll" |
    Measure-Object |
    ForEach-Object -Property Count
TessellatingHeckler
  • 27,511
  • 4
  • 48
  • 87