patternshellMinor
FizzBuzz: A PowerShell Story
Viewed 0 times
powershellstoryfizzbuzz
Problem
After reading way too much into what fizzbuzz was I though PowerShell could easily have its way with this.
What we have is a function that accepts pipeline input. For each number that is passed we run it through a switch. Be default it will check against both clauses. So when it hit 15 it will run for Fizz and Buzz. If neither are matched then the number just continues on without being converted. Since FizzBuzz must appear on the same line we capture the
Sample Output
Not sure what could be done beyond this. I suppose you could argue that a filter as supposed to a function would be more terse. I leave that to a reviewer to chew on.
Function Get-FizzBuzz{
param(
[Parameter(Position=0,ValueFromPipeline=$true)]
[int]$Number
)
process{
$result = switch($Number){
{$_ % 3 -eq 0}{"Fizz"}
{$_ % 5 -eq 0}{"Buzz"}
default{$_}
}
-join $result
}
}What we have is a function that accepts pipeline input. For each number that is passed we run it through a switch. Be default it will check against both clauses. So when it hit 15 it will run for Fizz and Buzz. If neither are matched then the number just continues on without being converted. Since FizzBuzz must appear on the same line we capture the
$result and use -join to concatenate the elements. This only has an effect on the $result when it has more than element. Sample Output
PS C:\Windows\system32> Get-FizzBuzz -Number 15
FizzBuzz
PS C:\Windows\system32> 1..15 | Get-FizzBuzz
1
2
Fizz
4
Buzz
Fizz
7
8
Fizz
Buzz
11
Fizz
13
14
FizzBuzzNot sure what could be done beyond this. I suppose you could argue that a filter as supposed to a function would be more terse. I leave that to a reviewer to chew on.
Solution
Your code looks good to me. I like how it takes pipelined input. That is pretty elegant.
You don't need the
This is what the help says about that:
By default, all function parameters are positional. Windows PowerShell assigns position numbers to parameters in the order in which the parameters are declared in the function.
The following comments are just my personal preferences. If this were a real code review for production code, I wouldn't insist on any of it. Feel free to ignore!
I think the
I'm almost tempted to say that multiple selection in switch statements is a misfeature of PowerShell because it violates the principle of least surprise. I'm torn on that question however because it is a cool feature.
Personally I would have used two
Regarding this line:
That is correct according to the help.
You don't need the
Position=0 here:[Parameter(Position=0,ValueFromPipeline=$true)]This is what the help says about that:
By default, all function parameters are positional. Windows PowerShell assigns position numbers to parameters in the order in which the parameters are declared in the function.
The following comments are just my personal preferences. If this were a real code review for production code, I wouldn't insist on any of it. Feel free to ignore!
I think the
switch statement there is elegant but a little confusing. In most languages, switch will only ever select a single branch. When I see "switch", I expect it to select one branch only. That's just a habit of thought. Your code took me by surprise. I had to stop and think about how it managed to get both "Fizz" and "Buzz" for the number 15.I'm almost tempted to say that multiple selection in switch statements is a misfeature of PowerShell because it violates the principle of least surprise. I'm torn on that question however because it is a cool feature.
Personally I would have used two
if statements. Even though that would have been clunklier, it would have been clearer and more explicit in my opinion, and therefore better.Regarding this line:
-join $resultThat is correct according to the help.
-join can indeed be a unary operator. It looks weird though. I'd never seen it like that before, and it took me by surprise. Personally I would have used it as a binary operator in the usual fashion:$result -join ""Code Snippets
[Parameter(Position=0,ValueFromPipeline=$true)]-join $result$result -join ""Context
StackExchange Code Review Q#116743, answer score: 5
Revisions (0)
No revisions yet.