写FizzBuzz
首先，我已经提出了这个问题。但我得到了不同的解决方案，在某些方面“更好”。目标是改进这个特定版本，而不是创建另一个版本。
以下是它的内容：
阅读编码恐怖，我刚刚偶然看到了另一次FizzBuzz。最初的帖子是这里。
我刚开始把它写下来。这只是一分钟的工作，但有几件事我不喜欢。
public void DoFizzBuzz()
{
var combinations = new Tuple<int, string>[]
{
new Tuple<int, string> (3, "Fizz"),
new Tuple<int, string> (5, "Buzz"),
};
for (int i = 1; i <= 100; i++)
{
bool found = false;
foreach (var comb in combinations)
{
if (i % comb.Item1 == 0)
{
found = true;
Console.Write(comb.Item2);
}
}
if (!found)
{
Console.Write(i);
}
Console.Write(Environment.NewLine);
}
}所以我的问题是：
：
- 如何摆脱
bool found？ - 有比
foreach更好的测试方法吗？
回答 6
Code Review用户
发布于 2012-08-03 10:11:15
我认为，如果您要设置一组数据来迭代，您应该开始更多地考虑功能。为什么不使用一组函数并对它们进行聚合呢？这将逻辑放在函数中，而不是在使用数据的函数中。
例如：
static void Main(string[] args)
{
var generators = new Func<string, int, string>[]
{
(s, i) => i % 3 == 0 ? s + "Fizz" : s,
(s, i) => i % 5 == 0 ? s + "Buzz" : s,
(s, i) => s ?? i.ToString()
};
var results = Enumerable.Range(0, 100).Select(i => generators.Aggregate((string)null, (s, f) => f(s, i))).ToList();
results.ForEach(Console.WriteLine);
}Code Review用户
发布于 2012-08-01 22:25:52
显然，您的解决方案偏离了我所称的“干净而简单”的方法。从你的描述中，我理解它，就好像这就是你解决问题的方法，因为有人要求你在面试时这样做。
我坚信，对于这个问题，您的解决方案太复杂了，而且对于这个问题未来将如何发展，您已经做了太多的假设。这样做有悖于称为KISS的软件原则(让它变得简单愚蠢)，而且由于这些“假设”，您更难阅读、理解和更改代码。
我将发布我认为更适合这个问题的原始/清洁解决方案，然后我将对为什么我认为“干净”解决方案比您的解决方案更好的原因进行简短的描述。
清洁解决方案：
for(int i=0; i <= 100; ++i) {
string s = ""
if(i%(5*3) == 0) {
s = "FizzBuzz";
} else if (i % 3 == 0) {
s = "Fizz";
} else if (i % 5 == 0) {
s = "Buzz";
} else {
s = i.ToString();
}
Console.WriteLine(s)
}在干净的解决方案中，可以清楚地看到正在发生的事情以及我们检查的值(甚至是5*3-值)。在您的解决方案中，您隐藏了这段代码中唯一有趣的东西(将字符串替换为数值)。
通过将值和字符串放置到不必要的元组对象中，并将这些重要值隐藏在称为comb.item1和comb.item2的东西后面，将更难理解正在发生的事情。
唯一有意义的解决方案是，如果知道下一次迭代需要以类似方式进行25次替换(7="Foo“、11="Bar”、77="FooBar")。但是，您仍然会遇到问题，必须以另一种方式例如3_5_7来处理“多重匹配”问题。
编程的整个想法是保持代码尽可能简单/干净(好书: Martin的“干净的Code_”)，避免在不知道下一步会发生什么的情况下做出“智能解决方案”。编写过于复杂的解决方案会让招聘人员发出警告。
您的解决方案可以轻松地添加更多的替换，但是下一次迭代最好是：
用“bazzinga”替换任何可以分割的部分，然后就不用理会嗡嗡声和嗡嗡声了。除非程序在周日运行，否则星期日是大家都知道的“星期日快乐”，字符串应该是“FizzySundayBuzz”或“FizzySunday”。
Code Review用户
发布于 2012-08-09 23:21:26
首先，我认为Daniel MesSer提出了一些优点，尽管如果您想要的是更简单、更易读的东西，我认为这种方法更容易读懂：
for(int i = 1; i <= 100; i++)
{
string output = null;
if (i % 3 == 0)
{
output += "Fizz";
}
if (i % 5 == 0)
{
output += "Buzz";
}
if (output == null)
{
output = i;
}
Console.WriteLine(output);
}虽然可以用?替换最后一个if ?？操作符，在我看来，这是最易读的版本。
如果有令人信服的理由引入一个表，我们可以或多或少地使用相同的解决方案。这个解决方案的优点是我已经去掉了bool，但是我使用了一个变量来保存每个数字的打印输出。该解决方案的另一个好处是，我只需要调用一个Console.WriteLine，而不需要在逻辑中多次调用Console.Write / WriteLine。
var combinations = new Tuple<int, string>[]
{
new Tuple<int, string> (3, "Fizz"),
new Tuple<int, string> (5, "Buzz"),
};
for (int i = 1; i <= 100; i++)
{
string output = null;
foreach (var comb in combinations)
{
if (i % comb.Item1 == 0)
{
output += comb.Item2;
}
}
if (output == null)
{
output = i.ToString();
}
Console.WriteLine(output);
}您可以摆脱foreach，用LINQ表达式替换它，但我怀疑大多数人会发现它更容易阅读。
var combinations = new Tuple<int, string>[]
{
new Tuple<int, string> (3, "Fizz"),
new Tuple<int, string> (5, "Buzz"),
};
for (int i = 1; i <= 100; i++)
{
string output = combinations.Where(combination => i % combination.Item1 == 0)
.Aggregate((string)null, (sum, value) => sum += value.Item2);
if (output == null)
{
output = i.ToString();
}
Console.WriteLine(output);
}为了好玩，为什么不把整个东西变成一个LINQ表达式，使其不可读：
var combinations = new Tuple<int, string>[]
{
new Tuple<int, string> (3, "Fizz"),
new Tuple<int, string> (5, "Buzz"),
};
Enumerable.Range(1, 100)
.Select(i => combinations.Where(combination => i % combination.Item1 == 0)
.Aggregate((string)null, (sum, value) => sum += value.Item2) ?? i.ToString())
.ToList()
.ForEach(Console.WriteLine);https://codereview.stackexchange.com/questions/14216
复制相似问题
腾讯云开发者
