Single line FizzBuzz solution in LINQ

Question

Now FizzBuzz itself isn't a big challenge but I agree that it can be a good tool to see if someone can code or not. I wanted to practice my LINQ a little bit so here's my single line FizzBuzz solution.

Enumerable.Range(1,100).Select(
                        n => 
                        (n % 15 == 0) ? "FizzBuzz" : 
                        (n % 3 == 0) ? "Fizz" : 
                        (n % 5 == 0) ? "Buzz" : 
                        n.ToString())
                        .ToList()
                        .ForEach(Console.WriteLine);

Answer 1

Overall, I like this. It is concise, and neat, and all. My only beef is the nested ternaries.

In general, ternary operator precedence is complicated by the lack of blocks.... The need for the parenthesis on the modulo precedence makes the conditions clear ( (n % 15 == 0) ) and the actual indenting you have, makes the logic a little clearer, but precedence is not that simple, and I have seen bugs where subtle precedence problems have caused the wrong execution-path through the nested ternaries....

.... bottom line, is for simple statements, ternaries are great, but the readability is compromised when there are multi-levels of ternaries. Nice for small things, ugly for big.

I would have preferred if you made the coditionals more obvious ... by extracting to a function maybe... but that would ruin your 1-liner.....

How about ... (Shown in this ideone here):

Enumerable.Range(1,100).Select(
                    n => 
                    (n % 15 == 0) ? "FizzBuzz" : 
                    (
                        (n % 3 == 0) ? "Fizz" :
                        (
                            (n % 5 == 0) ? "Buzz" :  n.ToString())
                        )
                    )
                    .ToList()
                    .ForEach(Console.WriteLine);

---

EDIT: The above answer is not as good as I wanted it to be.....

The exact problem with ternary precedence is that it is right-associative. This makes the logic potentially buggy, and hard to understand.....

Consider the ternaries:

expa = boola ? expc : expd;
expb = boolb ? expe : expf;

result = boolc ? expa : expb;

You can modify this to be:

expa = boola ? expc : expd;

result = boolc ? expa : boolb ? expe : expf;

Or, rewriting it in 'your' style, as:

expa = boola ? expc : expd;

result = boolc ? expa :
         boolb ? expe :
         expf;

But, you cannot 'inline' expa in the above as easily:

expb = boolb ? expe : expf;

result = boolc ? (boola ? expc : expd) : expb;

You need the parenthesis.

Inlining ternaries is just plain ugly.

---

Better solution

I regret suggesting to use the parenthesis to block the code in a more readable format. There is no more readable format, other than to extract the function, which is the 'right' thing to do:

    static private string FizzBuzz(int value)
    {
        if (value % 15 == 0)
        {
            return "FizzBuzz";
        }
        if (value % 3 == 0)
        {
            return "Fizz";
        }
        if (value % 5 == 0)
        {
            return "Buzz";
        }
        return value.ToString();
    }



    public static void Main()
    {
        foreach(var val in Enumerable.Range(1, 100).Select(n => FizzBuzz(n)))
            Console.WriteLine(val);
    }

Note, I have also undone the slow, and unnecessary conversion of the Enumerable to the ToList().ForEach() operation. Converting it to the list before re-converting it to an Enumeration is a waste of effort.

Bottom line, is that the 1-liner is not the best, or most readable format.

I have put this in an ideone, to show it working.

Answer 2

Your code is nice, relatively simple and a good attemp at LINQ - I've written very similar code before. However I have the following critiques:

• A list is created unnecessarily, eagerly loading the data. LINQ loves being deferred and streamed when possible - what happens if we never execute the code? What happens if we decide that we only need the first 50 results?
• The foreach method, while I like it, is considered poor code - LINQ shouldn't rely upon side effects, and the foreach keyword is roughly the same length.
• You're not using the LINQ paradigm - merely using the Enumerable and Select as a loop construct. This is why you have to use the Ternary if. Where is the Query? What are your data collections? And most importantly is it declarative - are you asking what you want rather than telling the computer how to get there?

Keeping within the '1LOC' idea:

foreach (var result in from number in Enumerable.Range(1, 100)
                       let match = (from condition in new[] 
                                    {   
                                        new {Key =15, Format= "FizzBuzz" },
                                        new {Key =5, Format= "Buzz" },
                                        new {Key =3, Format= "Fizz" },
                                        new {Key =1, Format= "{0}" }
                                    }
                                    where number % condition.Key == 0
                                    select condition.Format).First()
                       select string.Format(match, number))
    Console.WriteLine(result);
Console.ReadLine();

Why This Way?

I spent a fair bit of time coming up with this particular example - though its possible improvements could be made. Since this question and this answer have some interest, I'd explain why I chose this format.

My decision is simple: I want to use the LINQ Paradigm as much as possible, but want to stick within the artificial 1LOC requirement (otherwise I'm not really answering the question).

Let's break it down into steps:

Get a collection (0 to infinity) of values.
Get from a collection (0 to infinity) of 'conditions' 
   the format of the first condition where 'key' matches the value
Select the value formatted by the format.

Now the cool thing is that this basic structure is very generic. For example the collection of values can be XML, or maybe an infinite IEnumerable. The conditions could be a simple array, or a remotely hosted database. That's not forgetting that 'value' and 'condition' can be defined in many different ways - e.g. value could be an exception log and condition could be a function that formats or raises alerts based said exception.

foreach (var result in from log in Log.LastRun()  //gets log entries
                       let match = (from logHandler in Log Handling.Current  //finds appropriate handlers
                                    where log.Type == logHandler.Key
                                    select logHandler.Handler).First()
                       select match(log).Result)  //handle log - e.g. send SMS, alert user, create statistics, do nothing
    Console.WriteLine(result);  //E.g. 'Successfully SMS'd Error to User'
Console.ReadLine();

Of course, returning to our simple number variant, LINQ can be useful for making our problem easier to modify. For example, I require a version of FizzBuzz that prints "Douglas Adams" on mod 42 == 0, and will give me values 1 through to Infinity - and will ignore all values between 600 and 700.

foreach (var result in (from number in Counter(start: 1)
                       where number < 600 || number > 700
                       let match = from condition in new[] 
                                    {   
                                        new {Key =42, Format= "Douglas Adams" },
                                        new {Key =15, Format= "FizzBuzz" },
                                        new {Key =5, Format= "Buzz" },
                                        new {Key =3, Format= "Fizz" },
                                        new {Key =1, Format= "{0}" }
                                    }
                                    where number % condition.Key == 0
                                    select condition.Format).First()
                       select string.Format(match, number))
    Console.WriteLine(result);
Console.ReadLine();

Answer 3

Here's a variation on NPSF3000's example, but using method syntax and a Dictionary instead of a Tuple[]:

Enumerable.Range(1, 100)
.Select(n => new Dictionary<int, string>
    { {15, "FizzBuzz"}, {3, "Fizz"}, {5, "Buzz"}, {1, n.ToString()} }
    .First(kv => n % kv.Key == 0).Value)
.ToList()
.ForEach(Console.WriteLine);

Update: as noted in the comments, Dictionary doesn't guarantee order, even though this example works correctly on my machine. So if you use this KeyValueList implementation, you can replace Dictionary<int, string> with KeyValueList<int, string> and still use the dictionary initializer syntax. I prefer this to using the SortedDictionary since it doesn't change the ordering shown in code and therefore doesn't require using Last() instead of First().

Answer 4

I've used some answers here with a little refactoring, so you can easily add more numbers to divide:

var dic = new Dictionary<int, string> { { 3, "Fizz" }, { 5, "Buzz" } };

var t = 
        from number in Enumerable.Range(1, 100)
        let names = dic.Where(kv => number % kv.Key == 0).Select(kv => kv.Value)
        select names.Any() ? string.Join("", names) : number.ToString();

t.ToList().ForEach(Console.WriteLine);

Notice that it concatenates all the names that a number has, if it doesnt have any, it returns the number.

Answer 5

It's not necessary to extract the function. A multi-statement lambda will suffice:

Enumerable.Range(1,100).Select(n => {
    if (n % 3 == 0) {
      return n % 5 == 0 ? "FizzBuzz" : "Fizz";
    }
    return n % 5 == 0 ? "Buzz" : n.ToString();
}).ForEach(Console.WriteLine);

I prefer to use my own ForEach extension method rather than put the whole collection into a List:

public static void ForEach<T>(this IEnumerable<T> collection, Action<T> fn)
{
    foreach(T item in collection) { fn(item); }
}