我是C ++的新手(大约4周,没有其他语言的编程经验),并且正在编写一个程序,该程序应该执行以下操作:类型为float的数组。
定义类型为int的20个斐波那契数的数组。
使用重载函数计算包含100个数字的数组的均值。
使用重载函数计算标准包含100个数字的数组的偏差。
使用重载函数计算包含20个斐波那契数字的数组的平均值。
使用重载函数计算包含20个斐波那契数字的数组的标准偏差。 br />


#include "stdafx.h"
#include <iostream>
#include <cmath>
using namespace std;

const int SIZE_OF_GENERIC_ARRAY = 100;
const int SIZE_OF_FIBONACCI_ARRAY = 20;

double arithmeticAverage;
char sequenceType;

float Array[SIZE_OF_GENERIC_ARRAY];
int Fibonacci[SIZE_OF_FIBONACCI_ARRAY];

void fillGenericArray(int SIZE_OF_GENERIC_ARRAY);
void fillFibonacciArray(int SIZE_OF_FIBONACCI_ARRAY);

double mean(float[], int);
double mean(int[], int);

double standardDeviation(float[], int);
double standardDeviation(int[], int);

void outputMean();
void outputStandardDeviation();

int _tmain(int argc, _TCHAR* argv[])
{
   cout << "Would you like to generate a generic sequence or a Fibonacci sequence?"
     << endl
       << "\n"
       << "Type [G] + [ENTER] for a generic sequence, or;" << endl
       << "Type [F] + [ENTER] for a Fibonacci sequence: ";
   cin >> sequenceType;

    if (sequenceType == 'G' || sequenceType == 'g')
   {
       fillGenericArray(SIZE_OF_GENERIC_ARRAY);
       outputMean();
       outputStandardDeviation();
}

   else if (sequenceType == 'F' || sequenceType == 'f')
   {
       fillFibonacciArray(SIZE_OF_FIBONACCI_ARRAY);
       outputMean();
       outputStandardDeviation();
   }

   else
       cout << "\n"
            << "Invalid input. Please type 'F' or 'G'. Thank you.";

   return 0;
}

void fillGenericArray(int SIZE_OF_GENERIC_ARRAY)
{
   int i = 0;

   Array[0] = { 1 };

   for (int i = 0; i < SIZE_OF_GENERIC_ARRAY; i++)
       Array[i] = i + 1;

   return;
}

void fillFibonacciArray(int SIZE_OF_FIBONACCI_ARRAY)
{
   int i;

   Array[0] = { 0 };
   Array[1] = { 1 };

   for (int i = 2; i < SIZE_OF_FIBONACCI_ARRAY; i++)
       Array[i] = Array[i - 1] + Array[i - 2];

   return;
}

double mean(float Array[], int SIZE_OF_GENERIC_ARRAY)
{
   double sumOfElements = 0;
   int i;

   for (i = 0; i < SIZE_OF_GENERIC_ARRAY; i++)
   {
       sumOfElements += Array[i];
   }
   arithmeticAverage = sumOfElements / i;
   return (arithmeticAverage);
}

double mean(int Array[], int SIZE_OF_FIBONACCI_ARRAY)
{
   double sumOfElements = 0;
   int i;

   for (i = 0; i < SIZE_OF_FIBONACCI_ARRAY; i++)
   {
       sumOfElements += Array[i];
   }
   arithmeticAverage = sumOfElements / i;
   return (arithmeticAverage);
}

double standardDeviation(float Array[], int SIZE_OF_GENERIC_ARRAY)
{
   double standardDeviation;
   double tempSum = 0;

   for (int i = 0; i < SIZE_OF_GENERIC_ARRAY; i++)
   {
       tempSum += pow((Array[i] - mean(Array, SIZE_OF_GENERIC_ARRAY)), 2);
   }
   standardDeviation = sqrt(tempSum / (SIZE_OF_GENERIC_ARRAY));
   return (standardDeviation);
}

double standardDeviation(int Array[], int SIZE_OF_FIBONACCI_ARRAY)
{
   double standardDeviation;
   double tempSum = 0;

   for (int i = 0; i < SIZE_OF_FIBONACCI_ARRAY; i++)
   {
       tempSum += pow((Array[i] - mean(Array, SIZE_OF_FIBONACCI_ARRAY)), 2);
   }
   standardDeviation = sqrt(tempSum / (SIZE_OF_FIBONACCI_ARRAY));
   return (standardDeviation);
}

void outputMean()
{
   cout << "\n";
   cout << "The mean is: " << mean(Array, SIZE_OF_GENERIC_ARRAY);
   cout << endl;
}

void outputStandardDeviation()
{
   cout << "\n";
   cout << "The standard deviation is: " << standardDeviation(Array,
    SIZE_OF_GENERIC_ARRAY);
   cout << endl;
}


此代码编译并输出正确的结果。但是,我觉得我已经写了很多代码并有一些冗余。

我是否正确地应用了重载的概念?有没有更好的方法可以从mean()standardDeviation()函数内部调用void outputMean()void outputStandardDeviation()函数?有没有一种方法可以简化其中任何一个并使其更有效?

评论

欢迎使用CodeReview!这是一个很好的问题,感谢您抽出宝贵的时间来编写它,以便我们可以帮助您显示正确的编码样式和技术。我们都期待看到您的更多帖子!

我以为这段代码提出了很多有趣的观点,所以我在博客上写了:lesinskis.com/code_repair_02a_fibonacci_homework.html

#1 楼

我发现了几件可以帮助您改善代码的方法。为避免。
避免使用全局变量
using namespace std作为全局变量而不是using namespace std可能会有些道理。通常最好显式传递函数需要的变量,而不要使用模糊的全局变量隐式链接。
显式传递所需的变量
这基本上是陈述上一项的另一种方法。例如,您有一个函数SIZE_OF_GENERIC_ARRAY,该函数采用一个整数参数,该参数是数组的大小。很好,但是还应该将数组填充为另一个参数。将其付诸实践,我们得到:
void fillGenericArray(float array[], int arraysize)
{
   for (int i = 0; i < arraysize; ++i)
       array[i] = i + 1;
}

也请注意,第一个元素的初始化已在循环内完成,而无需显式完成。相同的功能
您的两个版本的arithmeticAverage相同,除了一个版本为fillGenericArrray()的数组,另一个版本为mean的数组。您可以使用模板来简化此操作:
template<typename T>
double mean(T array[], int arraysize)
{
   double sumOfElements = 0;
   for (int i = 0; i < arraysize; ++i)
       sumOfElements += array[i];
   return sumOfElements / arraysize;
}

还请注意,我还做了其他一些更改。更改了传递变量的名称
消除了float局部变量
从返回语句中删除了假括号
将循环变量从后递增更改为预递增

预先计算循环不变式
循环不变式是不变的循环构造中使用的值。现在,您的代码将为int函数中i循环的每次迭代重新计算arithmeticMean。由于它不会改变,因此最好只计算一次,然后在循环中重复使用它。结合上一点的思想,这两个函数就变成了单个模板化函数:然后在每个mean循环的范围内重新定义它。您可以(并且应该)消除每个函数顶部的for的冗余声明。另外,请注意,如果知道如何执行此编译器,那么您的编译器足够聪明,可以帮助您找到此类问题。计算并输出平均值。最好做这样的事情:
template<typename T>
double standardDeviation(T array[], int arraysize)
{
   double tempSum = 0;
   double mymean = mean(array, arraysize);
   for (int i = 0; i < arraysize; ++i)
   {
       tempSum += pow((array[i] - mymean), 2);
   }
   return sqrt(tempSum / arraysize);
}

或者,因为它现在非常简单,所以完全消除该函数,然后从standardDeviation内部调用,就像这样:同样,这也消除了您现有代码中的错误-无论选择哪个选项,ifillGenericArray都始终计算fillFibonacciArray的均值,而从不计算for的均值。 br />发出i实际上会刷新流并发出换行符。这会花费额外的时间和代码,而这些代码实际上并不需要。而是在此特定代码中的所有情况下都使用outputMean()
更新:
我注意到该代码对全局变量,局部变量和函数参数使用相同的名称。这使事情变得混乱,最好避免。例如,您的代码具有全局变量:
    std::ostream& outputMean(std::ostream &out, double mean)
    {
        return out << "\nThe mean is: " << mean << '\n';
    }

然后在main()中将其用作:
    std::cout << "\nThe mean is: " << mean(Array, SIZE_OF_GENERIC_ARRAY) << '\n';

该函数的代码是:
const int SIZE_OF_FIBONACCI_ARRAY = 20;

问题是这里有两个不同的东西叫做outputMean(),我想区别可能会使您感到困惑。让我们稍微重写一下该函数以使其更易于讨论:
fillFibonacciArray(SIZE_OF_FIBONACCI_ARRAY);

当此函数称为outputStandardDeviation()时,形式参数Array被实际参数Fibonacci取代,该参数实际在全局值中声明为之20,使您的原始代码感到困惑的是,有一个名为std::endl的形式参数,然后获得了实际具有相同名称的实际参数。不幸的是,代码然后引用了\n,它是一个全局变量。结果是,您的代码实际上没有在std::endl数组中创建一系列20个\n值,而是在main的前20个插槽中放置了20个SIZE_OF_FIBONACCI_ARRAY值,并且从未使用过fillFibonacciArray(SIZE_OF_FIBONACCI_ARRAY)数组。请参阅Wikipedia上的本文,以获取对形式参数和实际参数及其含义的更长时间和更完整的解释。

评论


\ $ \ begingroup \ $
我完全错过了i的双重声明。好答案
\ $ \ endgroup \ $
–马拉奇♦
2014年7月2日在15:39

\ $ \ begingroup \ $
该术语是“避免全局可变状态”。全局常数很好,不会引起任何问题。全局可变状态是导致问题的原因。
\ $ \ endgroup \ $
–马丁·约克
2014年7月2日在18:10

\ $ \ begingroup \ $
@LokiAstari:我同意全球可变状态更是一个问题。但是,在我看来,完全避免全局变量(包括常量)通常是个好建议。它们会污染全局名称空间,并且如果不非常谨慎地使用它们,可能会在大型程序中引起恼人的冲突。在这种特殊情况下,代码很容易用那些常量移动到main或至少声明为static的常量来重写,以将它们限制在文件范围内。
\ $ \ endgroup \ $
–爱德华
2014年7月2日在18:55

\ $ \ begingroup \ $
您反对全局静态const的论点不好。他们不需要污染全局名称空间,就可以轻松地将它们放置在特定的名称空间中并且仍然是全局的。全局常量在技术上没有任何问题(如果鼓励使用)。谷歌著名的谷歌编程谈论“全局可变状态”。
\ $ \ endgroup \ $
–马丁·约克
2014年7月2日,19:48



\ $ \ begingroup \ $
通常,“对几乎相同的功能使用功能模板”是一个很好的建议,但是教师希望看到重载的功能,而不是模板化的功能。
\ $ \ endgroup \ $
–雪体
2014年7月2日在20:26

#2 楼

是的,代码很糟糕,是的,您使用了不正确的重载,但是对于我来说,很难理解如何正确使用项目规范而不使用它。如果您的讲课内容是您的讲课的,您的讲师一定很糟糕。 std充满了越来越多的具有通用名称的不受约束的模板算法。编写std::cout而不是cout的价格不算什么,让编译器选择std::distance而不是您自己的distance的价格会让您哭泣。

名称通常是为宏保留的。您不想知道与宏冲突时会发生什么。此外,这些称为“魔术缓冲区大小”-它们是常量,没有逻辑或原因。您绝对应该评论为什么需要这些特定大小。

using namespace std;


永远不要在命名空间范围内声明可变变量。他们是最严重的罪过。它们很糟糕,无法控制,并且以后很难清理。

不要使用C样式的数组。它们之所以与B或BCPL兼容,并不是因为它们的行为良好。不是。它们非常不安全。您可以改用std::array<float, size>std::array<int, size>std::array生活在<array>中,其行为就像一个实值类型和标准容器。除非您知道这些东西实际上是什么,否则请使用_tmain。您也不需要main废话,尽管需要一点知识才能说服Visual Studio对此进行关闭。

现在,正如您已经正确注意到的那样,您的stdafxmean函数是完全重复的。 float和int版本。您可以轻松地将它们重新实现为两者的一个模板功能,但是也可以使用standardDeviation<algorithm>中存在的适当标准算法轻松定义它们。是毫无价值的垃圾,它们的逻辑应该只是在<numeric>中(而且,即使用户要求使用Fib,他们也不总是使用名称非常复杂的数组而不是Fibonacci数组吗?)

评论


\ $ \ begingroup \ $
我禁不住觉得您的回答听起来有些激进。我认为您的答案是完全正确的。
\ $ \ endgroup \ $
–西蒙·福斯伯格
2014年7月2日在15:28

\ $ \ begingroup \ $
老实说,Dead的回应让我有些失望。但是,这非常有启发性。我想学习这种语言。而且,如果这样做需要一些“坚强的爱”,那就这样吧... :)
\ $ \ endgroup \ $
–伊丽莎白(Elizabeth C.)
2014年7月2日在17:45

\ $ \ begingroup \ $
老实说,老师一点也不好。通常,我试图自己学习。我认为该网站是极好的资源。仅在一篇文章中,我至少学习了5到7个新概念。非常感谢!!
\ $ \ endgroup \ $
–伊丽莎白(Elizabeth C.)
2014年7月2日在18:22

\ $ \ begingroup \ $
但是要成为一名出色的老师,您需要首先教数组。因为出色的老师会告诉您“为什么” std :: array <>更好,并且这样做,您必须首先教给学生有关数组的知识,以便他们可以比较和对比差异。死记硬背地学习std :: array <>并不能帮助教育下一代程序员。
\ $ \ endgroup \ $
–马丁·约克
2014年7月2日在18:28



\ $ \ begingroup \ $
我也必须不同意outputX()函数是垃圾。这具有自我记录代码。命名好的函数可以替换一部分代码,从而可以通过自我记录自己的行为来使代码更易于维护。他们写得不好吗?是。但是您至少可以解释为什么它们写得不好(在我看来,因为它们仅使用std :: cout,我会对其进行参数化)。
\ $ \ endgroup \ $
–马丁·约克
2014年7月2日在18:32



#3 楼

您的代码通常是正确的,但是对标准偏差的计算效率很低。遍历数组。如果数组包含一百万个元素,则可能要花费一段时间。

更好的方法是一次调用mean

for (int i = 0; i < SIZE_OF_GENERIC_ARRAY; i++)
{
    tempSum += pow((Array[i] - mean(Array, SIZE_OF_GENERIC_ARRAY)), 2);
}


算法语言,您的解决方案是\ $ O(n ^ 2)\ $,而第二个版本是\ $ O(n)\ $。

有更高级的技术可以让您放弃使用全局变量和原始数组,但我知道您现在的目的是学习基础知识。

PS如果您数学上比较倾斜,还可以单次计算平均值和标准偏差。

评论


\ $ \ begingroup \ $
如果“更好”的意思是“编译器可以轻松地对其进行优化,但可读性较低”,那么我同意。
\ $ \ endgroup \ $
– DeadMG
2014年7月2日在14:37

\ $ \ begingroup \ $
您可能要在提出评论之前留出十秒钟以上的时间,因为实际撰写要花更多的时间。
\ $ \ endgroup \ $
– DeadMG
2014年7月2日在14:37

\ $ \ begingroup \ $
另外,我非常怀疑O(n ^ 2)对于现代硬件上的100个元素的浮点数组是否会成为问题。
\ $ \ endgroup \ $
– DeadMG
2014年7月2日在15:22

\ $ \ begingroup \ $
@DeadMG:那么您认为问题中的“有效”一词意味着什么?
\ $ \ endgroup \ $
– Beta
2014年7月2日在16:50

\ $ \ begingroup \ $
@DeadMG让我在这里闯入。我不同意。如果您直接看到可以更有效地执行某件事,那么它的编写效率就不高。除此之外,我发现“更正”的版本比原始代码更具可读性。此外,仅因为现在是100个元素,并不意味着它将永远是100个元素。想大
\ $ \ endgroup \ $
–Vogel612♦
2014年7月2日在18:41



#4 楼

您应该在循环中使用方括号时保持一致,前两个在一个衬里上不使用方括号,而其他人使用吗?我建议您始终使用方括号

for (int i = 0; i < SIZE_OF_GENERIC_ARRAY; i++)
    Array[i] = i + 1;




for (int i = 2; i < SIZE_OF_FIBONACCI_ARRAY; i++)
    Array[i] = Array[i - 1] + Array[i - 2];


对战

for (i = 0; i < SIZE_OF_GENERIC_ARRAY; i++)
{
    sumOfElements += Array[i];
}




for (int i = 0; i < SIZE_OF_GENERIC_ARRAY; i++)
{
    tempSum += pow((Array[i] - mean(Array, SIZE_OF_GENERIC_ARRAY)), 2);
}



还有其他Fluke格式问题,我确信与错别字或复印有关还要粘贴问题...

    if (sequenceType == 'G' || sequenceType == 'g')
   {
       fillGenericArray(SIZE_OF_GENERIC_ARRAY);
       outputMean();
       outputStandardDeviation();
}

   else if (sequenceType == 'F' || sequenceType == 'f')
   {
       fillFibonacciArray(SIZE_OF_FIBONACCI_ARRAY);
       outputMean();
       outputStandardDeviation();
   }

   else
       cout << "\n"
            << "Invalid input. Please type 'F' or 'G'. Thank you.";



if语句元素之间的回车
else语句上没有括号
if语句上的缩进

它应该是这样的

    if (sequenceType == 'G' || sequenceType == 'g')
    {
        fillGenericArray(SIZE_OF_GENERIC_ARRAY);
        outputMean();
        outputStandardDeviation();
    }
    else if (sequenceType == 'F' || sequenceType == 'f')
    {
        fillFibonacciArray(SIZE_OF_FIBONACCI_ARRAY);
        outputMean();
        outputStandardDeviation();
    }
    else
    {
       cout << "\n"
            << "Invalid input. Please type 'F' or 'G'. Thank you.";
    }



我不知道如何对其进行编码,但我认为可以做到,您应该将输入转换为大写,然后仅检查'G''F',这样可以消除额外的检查,但是我不知道要花多少钱。因此我会将输入转换为大写,然后取消检查小写字母。

sequenceType = std::toupper(sequenceType);
if (sequenceType == 'G')
{
    fillGenericArray(SIZE_OF_GENERIC_ARRAY);
    outputMean();
    outputStandardDeviation();
}
else if (sequenceType == 'F')
{
    fillFibonacciArray(SIZE_OF_FIBONACCI_ARRAY);
    outputMean();
    outputStandardDeviation();
}
else
{
   cout << "\n"
        << "Invalid input. Please type 'F' or 'G'. Thank you.";
}


评论


\ $ \ begingroup \ $
支撑风格是他最少的问题。这什么也没做,只是让他专注于完全错误的问题。
\ $ \ endgroup \ $
– DeadMG
2014年7月2日在15:22

\ $ \ begingroup \ $
我非常不同意,如果代码混乱且难以阅读,则很难发现问题,编写简洁的代码可以使调试更容易。
\ $ \ endgroup \ $
–马拉奇♦
2014年7月2日在15:26

\ $ \ begingroup \ $
+1,建议使用括号。不知道我是否同意过多的换行符。对于大小写转换,有std :: toupper()。从技术上讲,这在效率上会降低效率,但这不会有所作为。
\ $ \ endgroup \ $
– jliv902
2014年7月2日在15:28

\ $ \ begingroup \ $
没有很多换行符,但是值得一提的是,不需要std :: endl。首选“ \ n”。
\ $ \ endgroup \ $
– Jamal♦
2014年7月2日在15:32

\ $ \ begingroup \ $
@ jiv902:技术上效率较低。其实我不知道我是否会打赌。最简单的是数组访问操作。因此可以归结为:if(sequenceType =='F'|| sequenceType =='f')vs if(ToUpper [sequenceType] =='F')那是很难打电话的。因此,我将以最易读的方式作为论据。
\ $ \ endgroup \ $
–马丁·约克
2014年7月2日在18:21