Is this code acceptable? Is it the “Powershell way”?

Welcome Forums General PowerShell Q&A Is this code acceptable? Is it the “Powershell way”?

Viewing 4 reply threads
  • Author
    Posts
    • #208212
      Participant
      Topics: 86
      Replies: 138
      Points: 677
      Rank: Major Contributor

      I have a CSV file:

      Person,Amount,Item
      Joe,20,gas
      Tom,5,food
      Tom,10,drink
      

      and I want to show the most expensive spender and the associated item:

      $data = import-csv .\mycsvfile
      $biggest = $Null
      foreach ($x in $data) {
          if ($x.Amount -gt $biggest) { $biggest = $x }
      }
      $biggest | Format-table

      I did this because I couldn’t use Measure-object to show the Person and Item properties as well.  My questions:
      (1) Is there a better (shorter perhaps) alternative to my above solution?
      (2) Is my solution acceptable, i.e., the “Powershell way” of solving this problem?
      Would be grateful for any advice, suggestions or tips for improvements.

      • This topic was modified 5 months ago by Ramon TAN.
      • This topic was modified 5 months ago by Ramon TAN.
    • #208245
      Participant
      Topics: 5
      Replies: 2411
      Points: 6,193
      Helping Hand
      Rank: Community MVP

      Import-CSV returns strings. If you want to “calculate” with some these strings properly you would need to “convert” them to integers or other number types. Then you can use cmdlets like Sort-Object to sort them properly.

      Import-Csv -Path .\mycsvfile.csv | 
          Select-Object -Property Person,@{Name = 'Amount'; Expression = {$_.Amount -as [INT]}},Item |
              Sort-Object -Property Amount -Descending |
                  Format-Table -AutoSize

      And now you have a sorted list and you could do this:

      Import-Csv -Path D:\sample\mycsvfile.csv | 
          Select-Object -Property Person,@{Name = 'Amount'; Expression = {$_.Amount -as [INT]}},Item |
              Sort-Object -Property Amount -Descending |
                  Select-Object -First 1 |
                      Format-Table -AutoSize
    • #208263
      Participant
      Topics: 0
      Replies: 3
      Points: 40
      Rank: Member

      I’d suggest not to try to force your code in to a particular style of coding at the expense of readability.

      Despite Olaf’s brilliant example of solving the problem in the “powershell way”, I was able to determine what your code was trying to achieve more easily. Just my opinion anyway.

       

    • #208572
      Participant
      Topics: 2
      Replies: 513
      Points: 1,322
      Helping Hand
      Rank: Community Hero

      What we find readable largely depends on what code we’re used to reading. My own preference would be a bit different, and for a couple reasons:

      1. Sorting objects should probably happen first
      2. The Select-Object statements can then be combined.
      Import-Csv -Path '.\mycsvfile.csv' |
          Sort-Object -Property { $_.Amount -as [int] } |
          Select-Object -Property Person, Amount, Item -First 1

      Some additional reasons a pipeline might be preferred over a straight loop:

      1. It lets you stream input in and handle it a line at a time, without having to wait for the entire file to be imported first, so it’s usually lighter on memory usage overall.
      2. As a result, it can typically handle larger files better, and doesn’t necessarily need to load the entire file into memory — if you don’t need to compare things across the entire file (unlike here)

      That said… there’s not really anything wrong with your initial approach, beyond perhaps a few extremely minor formatting preferences, and your if condition probably might need a second look over.

       

    • #208638
      Participant
      Topics: 86
      Replies: 138
      Points: 677
      Rank: Major Contributor

      My sincerest thanks to all for their very educational and informative advice.  As a beginner to PS I have found this forum to be an extremely valuable source of useful knowledge.  The answers I have been getting such as the ones from this recent post remind me of some advice I read somewhere in the past — let Powershell do the work.  Indeed, my (foreach / if ) approach says I am still in need of help from experts like you  … thanks again !

Viewing 4 reply threads
  • The topic ‘Is this code acceptable? Is it the “Powershell way”?’ is closed to new replies.