关于编程、重构等 42条建议 下

上一篇：关于编程、重构等 42条建议 中

原文来自：
The Ultimate Question of Programming, Refactoring, and Everything
https://software.intel.com/en-us/articles/the-ultimate-question-of-programming-refactoring-and-everything

译文来自：http://blog.csdn.net/huanglong8/article/details/56668577

其实这42条建议可以很快过的，这估计有很多都是老外的习惯。。。

29. 在迭代器中使用++i,不要用后缀i++

来自虚幻4引擎项目，如果itr是迭代器，那使用++itr会更好。

void FSlateNotificationManager::GetWindows(....) const
{
  for( auto Iter(NotificationLists.CreateConstIterator());
       Iter; Iter++ )
  {
    TSharedPtr<SNotificationList> NotificationList = *Iter;
    ....
  }
}

说明
首先，译者认为++i的效率要比i++高，因为在看重载时你会尤为深刻的认识到这一点。

MyOwnClass& operator++()
{
  ++meOwnField;
  return (*this);
}

MyOwnClass operator++(int)
{
  MyOWnCLass tmp = *this;
  ++(*this);
  return tmp;
}

如今编译器可以优化，在release版本可能差异不大，但debug调试时，这种性能问题会稍显明显。

30. VC++和sprintf函数

此来自Energy Checker SDK

int main(void) {
  ...
  char *p = NULL;
  ...
  wprintf(
    _T("Using power link directory: %s\n"), 
    p
  );
  ...
}

说明
第一个错误是使用_T以宽字节构建字符串。使用L才是这里正确的方法。如果我们不使用宽字节格式，_T将不会扩展任何内容，代码也不会被编译。如果你使用wprintf打印，你应该使用格式%S。
问题很奇怪，但这是微软干的。
正确的代码

char *p = NULL;
...
#ifdef defined(_WIN32)
wprintf(L"Using power link directory: %S\n"), p);
#else
wprintf(L"Using power link directory: %s\n"), p);
#endif

建议
没啥好建议的，只是警告你这里会有问题，在VS2015中，有了可移植的兼容解决方案，就是给预处理器 _CRT_STDIO_ISO_WIDE_SPECIFIERS宏。
这时候代码变成

const wchar_t * p = L“abcdef”;
const char * x =“xyz”;
wprintf（L“％S％s”，p，x）;

这样就正确了，分析器知道这个宏的意思，并考虑所有代码。当然译者看到着，也就呵呵了，一脸懵逼继续。

31. C/C++中，数组参数传递问题

取自Wolf项目。sizeof要清除返回的大小是否是你想要的大小。

ID_INLINE mat3_t::mat3_t( float src[ 3 ][ 3 ] ) {
  memcpy( mat, src, sizeof( src ) );
}

返回的是指针的大小，而不是数组的大小。那这里永远都是4或8。。。
正确的代码

ID_INLINE mat3_t::mat3_t( float src[ 3 ][ 3 ] ) {
  memcpy(mat, src, sizeof(float) * 3 * 3);
}

建议
你可以通过引用数组来传递

ID_INLINE mat3_t::mat3_t( float (&src)[3][3] )
{
  memcpy( mat, src, sizeof( src ) );
}

嗯，这样是可以的。另一种方案就是使用array类等或stl。。。你可以继续参考 不要将单个指针作为数组传递

32. 危险的printf

取自TortoiseSVN项目
BOOL CPOFile::ParseFile(….)
{
….
printf(File.getloc().name().c_str());
….
}
说明
通常打印或写文件，程序员会这么干

printf（str）;
fprintf（file，str）;

这是一个非常不安全的写法，你没有格式化说明符，可能导致一些后果。译者表示，在实际开发中，确实遇到过，明明都是字符数组，却打印混乱。
需要额外注意的是，这是程序本身的一个真正的漏洞，它可以受黑客控制，以此来做其他的动作，所以要小心。
正确的代码

printf（“％s”，File.getloc（）。name（）。c_str（））;

建议
类似输出，流等需要格式化的东西，一定要做格式化操作，以免导致潜在的危险。

33. 永远不要引用空指针

此来自GIT源代码中

void mark_tree_uninteresting(struct tree *tree)
{
  struct object *obj = &tree->object;
  if (!tree)
    return;
  ....
}

说明
应用空指针是会造成错误的。所以不要试图偷懒。这里作者到时又说了一堆话，译者表示不翻了。有个更详细的解释：http : //www.viva64.com/en/b/0306/
建议
写好代码，关爱你我。

34. 保证类型的区间大小可用

这里作者没有说哪个项目，但主要说，如果你定义了一个int型的变量，在做循环时尤要保证循环的大小范围，一旦超出int范围，就会产生错误。

35. 添加枚举记得修改switch

来自Appleseed项目

enum InputFormat
{
    InputFormatScalar,
    InputFormatSpectralReflectance,
    InputFormatSpectralIlluminance,
    InputFormatSpectralReflectanceWithAlpha,
    InputFormatSpectralIlluminanceWithAlpha,
    InputFormatEntity
};

switch (m_format)
{
  case InputFormatScalar:
    ....
  case InputFormatSpectralReflectance:
  case InputFormatSpectralIlluminance:
    ....
  case InputFormatSpectralReflectanceWithAlpha:
  case InputFormatSpectralIlluminanceWithAlpha:
    ....
}

说明
这个也是由于程序员疏忽造成的，但时刻应该记得，switch中是有default的，这样可以避免你出错的机会。
修改

switch (m_format)
{
  case InputFormatScalar:
  ....
  case InputFormatSpectralReflectance:
  case InputFormatSpectralIlluminance:
  ....
  case InputFormatSpectralReflectanceWithAlpha:
  case InputFormatSpectralIlluminanceWithAlpha:
  ....
  case InputFormatEntity:
  ....
  default:
    assert(false);
    throw "Not all variants are considered"
}

如果有未定义的枚举，就会抛出异常，这有两个缺点。如果测试没测到，到了用户，那就麻烦了。如果考虑默认其他作为错误，那必须将所有枚举都写一个值，这不方便。作者再次建议这么写。

enum InputFormat
{
  InputFormatScalar,
  ....
  InputFormatEntity
  //If you want to add a new constant, find all ENUM:InputFormat.
};

switch (m_format) //ENUM:InputFormat
{
  ....
}

好吧，就是注释而已，搜代码，看来并没有特别好的方法。

36. 如果电脑发神经，请检查内存

这个问题译者也遇到过，当然作者遇到的可能是真正的问题。作者旨在说明，如果你的程序发生异常，奇怪的问题，不要怪编译器，不要怪pc，你应该试图去找到代码中的问题，因为99.99%的bug都出自代码，当然，也不排除其他可能，但那是很少见的。作者截了两个图。


在调试中，作者试图修改内存中的值，可敲入回车时，却发现变成了2.并且立刻去找内存测试程序进行确认。memtest86
后来作者替换了内存库表示一切正常了。。。虽然这没有什么技巧性的建议，但旨在告诉你，检查内存，确保无误。

37. 注意在do while中的continue

来自Haiku项目

do {
  ....
  if (appType.InitCheck() == B_OK
    && appType.GetAppHint(&hintRef) == B_OK
    && appRef == hintRef)
  {
    appType.SetAppHint(NULL);
    // try again
    continue;
  }
  ....
} while (false);

这段代码看似很傻，其实疏忽也会带来错误的根本。
说明
continue是循环跳出，并没有达到预想的效果。例如下面程序

for (int i = 0; i < n; i++)
{
  if (blabla(i))
    continue;
  foo();
}
Or like this:

while (i < n)
{
  if (blabla(i++))
    continue;
  foo();
}

这是没有问题的，可能程序员就是这么想的，但如果这样

do
{
  if (blabla(i++))
    continue;
  foo();
} while (i < n);

直觉上往往是错误的。事实证明do while(false)中，用continue相当于break。
正确的代码

for (;;) {
  ....
  if (appType.InitCheck() == B_OK
    && appType.GetAppHint(&hintRef) == B_OK
    && appRef == hintRef)
  {
    appType.SetAppHint(NULL);
    // try again
    continue;
  }
  ....
  break;
};

建议
作者的建议又是主观情感。。。

38. 从现在开始使用nullptr而不是null

好吧，译者表示这个已经知道了。。。

39. 代码怎么又不正确了

来自Miranda NG’s

#define MF_BYCOMMAND 0x00000000L
void CMenuBar::updateState(const HMENU hMenu) const
{
  ....
  ::CheckMenuItem(hMenu, ID_VIEW_SHOWAVATAR,
    MF_BYCOMMAND | dat->bShowAvatar ? MF_CHECKED : MF_UNCHECKED);
  ....
}

说明
仍然是优先级问题 ？：优先级要低于|操作。作者建议尽量避免复杂的表达式。。。译者表示重复的凌乱。。。

40. 使用静态代码分析

来自Haiku项目

int compareTypeAndID(....)
{
  ....
  if (lJack && rJack)
  {
    if (lJack->m_jackType < lJack->m_jackType)
    {
      return -1;
    }
    ....
}

说明
作者又强调谨慎，细致，但这不是慢慢习惯养成的么，关键中国这么多做外包的，你让外包情何以堪。。。
自己找错误。。。
作者还建议一些单元测试之类的。。。

41. 避免向项目中添加新库

作者意思表示，如果有这么一个功能，你是用了外部库，或者其他的第三方库，那么你就要做好同时维护的工作，你可以会发现 2017年的程序，还在用2002年的库，并且2002年的库存在重大网络漏洞，当升级后，又不兼容2017年的程序，这是可怕的一件事，可惜，在译者身上发生了，好在只是用了一小段功能。。。
不要过分使用库，作者提出几点想法。

• 添加新库，如果很重，则增加项目的大小。
• 即使使用1%的功能，他都包含了整体，负担很重。
• 一起编译很复杂，开发的体验并不是很好。
• 如果你关心安全漏洞，你就知道这是致命的。
• 你用的那个库如果更改了许可咋办。。。
• 升级也是很麻烦的事
• 不同编译器，移植，呵呵呵。。。
• 不同的平台也会有问题。
• 如果太多，我都不知道类叫什么名字，因为太多重名。。。
• 添加很多库没事，但很乱。
• 新库有很多bug，你可能都不知道。
  但不是说我们不能使用库，还是合理使用，不能滥用。
  建议
  不要急于添加库到项目中，你应该多思考，他的轻重优劣。

42. 不要使用empty命名函数

来自WinMerge项目

void CDirView::GetItemFileNames(
  int sel, String& strLeft, String& strRight) const
{
  UINT_PTR diffpos = GetItemKey(sel);
  if (diffpos == (UINT_PTR)SPECIAL_ITEM_POS)
  {
    strLeft.empty();
    strRight.empty();
  }
  ....
}

说明
作者表示，不好的命名会造成其他程序员阅读的误导。例如clear erase empty我们应该尽可能按其本身进行编码。就像empty我们应该认为它是返回是否为空的信息，而并非清空。。。译者表示，我的习惯还是很好的。

结论

这篇文章，或许帮助不是特别大，但至少会强调一些内容。我的目的也是时刻警告程序员，你的粗心可能会导致整个项目在找一个奇怪问题，而耗费了大量的时间，避免这些错误，上点心，这是我的建议。为此，祝大家可以进行无bug编码！

我也邀请大家关注Twitter @Code_Analysis