1
\$\begingroup\$

I have only started working with XML files and PowerShell. I feel that while there are many ways to accomplish the same thing that I might be doing the following inefficiently.

This is all based on the following SO post which I have an answer to:

$numberOfCores = Get-WmiObject -class win32_processor numberOfCores | Select-Object -ExpandProperty numberOfCores

$path = "C:\Windows\Microsoft.NET\Framework64\v4.0.30319\Config\machineTest.config"
[xml]$machineConfig = Get-Content $path

# Remove the elements we are going to be replacing
$node = $machineConfig.SelectNodes("/configuration/system.web") 
[void]($node.RemoveChild(($node.SelectSingleNode("processModel"))))
[void]($node.RemoveChild(($node.SelectSingleNode("httpHandlers"))))

# Create the element processModel and set attributes
$processModelxml = $machineConfig.CreateElement("processModel")
$processModelxml.setAttribute("maxWorkerThreads",370)
$processModelxml.setAttribute("maxWorkerThreads",370)
$processModelxml.setAttribute("maxIoThreads",370)
$processModelxml.setAttribute("minWorkerThreads",50)
$processModelxml.setAttribute("minIoThreads",50)
[void]($node.AppendChild($processModelxml))

# Create the element httpRuntime and set attributes. Adjust values based on number of cores
$httpRuntimexml = $machineConfig.CreateElement("httpRuntime")
$httpRuntimexml.setAttribute("minFreeThreads",90 * $numberOfCores)
$httpRuntimexml.setAttribute("minLocalRequestFreeThreads",80 * $numberOfCores)
[void]($node.AppendChild($httpRuntimexml))

# Build the <system.net> section
[xml]$systemnetxml = @"
  <system.net>
    <connectionManagement>
      <add address = "*" maxconnection = "$(200 * $numberOfCores)" />
    </connectionManagement>
  </system.net>
"@

# Import into config
$machineConfig.configuration.AppendChild($machineConfig.ImportNode($systemnetxml."system.net",$true)) | Out-Null

# Save changes
$machineConfig.Save("c:\temp\testing.xml")
# Change back to $path to write back to original file.

If you wanted to test this you need to change $path to that of a local machine.config in a .Net build folder. Also the last step saves the file in an alternate location for testing. Changing it to $machineConfig.Save($path) would be a production approach.

In my defense this code was gear towards a new user so there are a few lines that are created to be more verbose on purpose. I am mostly interested in the XML focus of this code but will welcome any criticism towards the rest (in case I am being complacent.) Was designed using PowerShell v4.

\$\endgroup\$
  • 1
    \$\begingroup\$ Hoping to encourage more PS questions here. I keep lurking to help but there are not that many. \$\endgroup\$ – Matt Jan 7 '16 at 17:03
1
\$\begingroup\$

You code looks fine to me. I'll just make some random comments.

You can split long lines like this for the sake of readability:

$numberOfCores = Get-WmiObject -class win32_processor numberOfCores |
    Select-Object -ExpandProperty numberOfCores

This comment is confusing to me:

# Remove the elements we are going to be replacing

You are removing <httpHandlers> but adding back <httpRuntime>. Was that intentional or a bug?

Why remove and create a new <processModel>? Why not just update the attributes in the original node? You would save on a few lines of code.

What happens if these nodes already have some attributes set? If you blow away the nodes, you will blow away those attributes. Was that intentional?

Why do you suddenly change style when creating <system.net>? Why not continue using CreateElement and so on?

In most places, you cast to [void] to get rid of the return values. In one place you use Out-Null. It's best to be consistent. Personally I would go with Out-Null since it is more the PowerShell way, and I think it looks nicer.

If you like, you can use the Select-Xml cmdlet instead of SelectNodes. Something like this:

$node = $machineConfig | Select-Xml /configuration/system.web

If you are going to overwrite the original file, it might be a good idea to make a copy of the original first. Add a timestamp to the filename or something.

It might be overkill in this case, but I would be inclined to make the code data-driven. That is, put all the changes to be done in a list at the top of the code and walk through the list in the code making the changes. That way people could see at a glance what the script does without having to read through all the code.

\$\endgroup\$
  • \$\begingroup\$ Good points in general. I usually split longer lines in my answers but that one is short enough on my console. I usually use [void] as I recall a post that showed it was faster. I use out-Null for answers as it is more PowerShelly. You are seeing me switch for this question but missing one! the runtime/handler thing was my mistake and removed the wrong thing. I have updated my answer in the linked question. Why do you suddenly change style because I knew how to add that block but was not sure how to add mass attributes. Looking at it again I think I have the answer. This was helpful \$\endgroup\$ – Matt Jan 8 '16 at 2:43
  • \$\begingroup\$ As for the last point I understand. I was coding in the order the OP requested changes. Something to watch for when I come here with questions. I never know if I should be using X-Path or not. Since I am not really searching in this case i cannot take advantage of it. \$\endgroup\$ – Matt Jan 8 '16 at 2:44
  • \$\begingroup\$ The several lines of adding attributes seemed inefficient. But I think I answered that with the last part of my code. \$\endgroup\$ – Matt Jan 8 '16 at 2:45
  • \$\begingroup\$ @Matt, what do you mean by efficiency? You should write code so that it's easy to read. Micro-optimizations are rarely ever needed. \$\endgroup\$ – Dangph Jan 8 '16 at 4:34
  • \$\begingroup\$ In my case I more meant I was curious if there was another, albeit readable, way to set multiple attributes at once. I could easily make the code longer but I would be sacrificing efficiency when in theory the lines could be reduced. That is why I used $systemnetxml as it was a way to make several changes at once without having to use several individual statements. \$\endgroup\$ – Matt Jan 8 '16 at 11:50

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.