4
\$\begingroup\$

I'm looking to improve a loop within a script that performs scheduled server restarts.

Currently, I'm using a DO While loop with an exit condition within an IF statement once a counter reaches a limit.

I'm looking to change this into a more simpler loop using a Labeled While loop containing a For loop that breaks when the nested If loop catches the counter variable reaching a limit. I've read it's best to avoid using Break but I just cannot come up with anything.

#current loop
$max = 60;$i = 0
 DO
 {
  IF($i -gt $max){
  [PsCustomObject]@{
  "Server" =  $server
  "Status" = "FailedToReboot!"
  "LastRebootTime" = "$LastReboot"
  "CurrentRebootTime" = "FailedToReboot!"
   }#end custom object

Send-MailMessage <Details>

exit}#exit script and log failed to reboot.

$i++

Start-Sleep -Seconds 10

}#end DO

While (Test-path "\\$server\c$")#After this point, server went offline, continue script.


#new test loop    
:WaitingForReboot while (test-path \\$server\C$){
 for($i = 0; $i -le 7){    
 $i++
 "Waiting for $server to reboot, waited " + $(10 * $i) + " of 50 seconds"

  IF($i -eq 5){"Exit wait loop";break WaitingForReboot}Else{
    #do nothing in Else, until $i hits 5.
    }#else block
Start-Sleep -Seconds 10        
 }#for loop
}#while loop
\$\endgroup\$
2
\$\begingroup\$

As you have read it is usually preferred to exit a loop via its condition(s) as supposed to force exiting via commands like break, exit and continue. Those commands do have their place but your loop and logic could do without them. You are only executing the code inside the if once. Why bother keeping it inside the loop? Lets update the while condition and move the if clause outside.

$maximumAttempts = 60
$attemptIndex = 0

do {
    # Wait between checks. 
    $attemptIndex++
    Start-Sleep -Seconds 10
} While (Test-path "\\$server\c$" -and $attemptIndex -le $maximumAttempts)

# Check if the loop ran for its maximumAttempts
if($attemptIndex -ge $maximumAttempts){

  [PsCustomObject]@{
      "Server" =  $server
      "Status" = "FailedToReboot!"
      "LastRebootTime" = "$LastReboot"
      "CurrentRebootTime" = "FailedToReboot!"
   }

   Send-MailMessage <Details>
}

You will also see that I have used more meaningful variable names. I have also used standardized indentation. Doing that removes the need for comments like }#while loop. The indentation makes it clear where the brackets start and stop and make reading the code easier.

\$\endgroup\$
  • \$\begingroup\$ I'm guessing the $max in the IF statement should be $maximumAttempts ? I actually went with an Until block and changed up the loop slightly. Also included a custom object bool flag that I use at the end of the script to re-check if needed. Should I update the question with the newer code? I do appreciate the pointers! \$\endgroup\$ – user4317867 Jul 4 '16 at 1:46
  • \$\begingroup\$ If the changes you make invalidate current answers it is discouraged. If you have newer, different code then by all mean make another question. I also updated the if statement as I forgot to update the variables. Thanks. \$\endgroup\$ – Matt Jul 4 '16 at 1:56
  • \$\begingroup\$ Will have to pick this up next time! \$\endgroup\$ – user4317867 Jul 4 '16 at 2:16

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.