Vigenere加密

Question

有人能在这里评论一下我的C++样品吗？

#include <iostream>   //console io
#include <string>     //string handling
#include <algorithm>  //transform

using namespace std;

struct to_upper {
  int operator() (int ch)
  {
    return toupper(ch);
  }
};

void encrypt_vigenere(const string& plaintext, const string& key, string& ciphertext){
   int i, j, ch;
   for(i = 0, j = 0; i < plaintext.length(); ++i, j++) {
      if(j >= key.length()) 
         j = 0;
      if(plaintext[i] >= 'A' && plaintext[i] <= 'Z')
         ch = ((plaintext[i] - 'A') + (key[j] - 'A')) % 26;
      else
         ch = plaintext[i] - 'A';  //only encrypt A-Z
      ciphertext.append(string(1, (char)(ch + 'A')));
   }
}

void decrypt_vigenere(const string& ciphertext, const string& key, string& plaintext){
   int i, j, ch;
   for(i = 0, j = 0; i < ciphertext.length(); ++i, j++) {
      if(j >= key.length()) 
         j = 0;

      if(ciphertext[i] >= 'A' && ciphertext[i] <= 'Z')
         ch = ((ciphertext[i] - 'A') + 26 - (key[j] - 'A')) % 26;
      else
         ch = ciphertext[i] - 'A';  //only decrypt A-Z

      plaintext.append(string(1, (char)(ch + 'A')));
   }
}


int main(int argc, char* argv[])
{
   cout << "Enter plaintext to encrypt:\n";
   char plaintext[256] = {0};
   char key[256] = {0};
   cin.getline (plaintext,256);
   cout << "Text to encrypt: " << plaintext << endl;

   cin.clear();  //clears any cin errors 

   cout << "Enter key (can be any string):";

   cin.getline (key, 256);

   //uppercase KEY
   transform(key, key+strlen(key), key, to_upper());

   cout << "key chosen: " << key << endl;

   string cipher;
   //uppercase plaintext
   transform(plaintext, plaintext+strlen(plaintext), plaintext, to_upper());
   encrypt_vigenere(plaintext, key, cipher);

   string decrypted;
   decrypt_vigenere(cipher, key, decrypted);

   cout << "Vigenere encrypted text: " << cipher << " decrypted: " << decrypted << endl;

return 0;
}

Answer 1

通用编码风格

请不要这样做：

using namespace std;

我知道每一本C++书籍都是以这一行开头的。很好，它适用于他们在书中显示的十行程序。但是当你开始写真正的程序时，这就变成了一种痛苦，因为它可能会带来冲突，因此真正的程序将不会包含在这些程序中。所以要习惯不使用它。

有几种选择：

• 以std作为前缀：：
• 使用using声明从命名空间中获取特定类型/对象
  • 要有选择性，把他们绑得尽可能紧。

已经有了一个由标准定义的toupper() (而且您正在使用它)。

struct to_upper {
  int operator() (int ch)
  {
    return toupper(ch);
  }
};

只需在转换中使用它(注意，您需要指定::toupper)。

 std::transform(begin, end, dst, ::toupper);

每行一个变量：

int i, j, ch;

并使它们具有可读性(您不需要删除它们并使它们成为100个字符)，但是当您试图在代码中发现它们的所有用途时，一个字符变量是一个痛苦。还要注意，您可以将循环变量声明为for()的一部分。尽可能地在使用点附近声明您的变量。

在你的用法上要保持一致：

for(i = 0, j = 0; i < plaintext.length(); ++i, j++) {

为什么它是++i，而您使用的是j++。它让我看代码，看看是否有什么特别的东西，我应该注意(似乎没有)。所以你在浪费我的时间让我觉得他为什么要这么做。

就我个人而言(这只是我愿意做的一件事，你可以接受，也可以离开)。我将取代这一点：

  if(j >= key.length()) 
     j = 0;

使用(显然也从for()中删除它)：

  // Increment and wrap `j`
  j = (j + 1) %  key.length();

很确定你的地窖不工作了。

  if(plaintext[i] >= 'A' && plaintext[i] <= 'Z')
     ch = ((plaintext[i] - 'A') + (key[j] - 'A')) % 26;
  else
     ch = plaintext[i] - 'A';  //only encrypt A-Z
  ciphertext.append(string(1, (char)(ch + 'A')));

如果要读取用户输入，则使用std::string。它以完全相同的方式工作，使事情变得更简单(您不需要检测和修复非常长的字符串)。

char plaintext[256] = {0};
cin.getline (plaintext,256);

// Easier as

std::string plaintext;
std::getline(std::cin, plaintext);

再次声明您的变量接近您将要使用它们的点。不要陷入将变量放在函数顶部的C样式中。

好的。所以您清除了错误状态。

cin.clear();  //clears any cin errors 

但你对此什么都不做。

这给了你一种虚假的安全感。最好避免。如果您要处理错误，请正确地处理或完全不处理。

算法使用

显然，您已经看到了标准库如何使用算法。

transform(key, key+strlen(key), key, to_upper());

还有其他几个地方可以使用标准算法来使代码更清晰。加密和解加密循环正是这种东西工作得很好的地方：

for(i = 0, j = 0; i < plaintext.length(); ++i, j++) {
  if(j >= key.length()) 
     j = 0;
  if(plaintext[i] >= 'A' && plaintext[i] <= 'Z')
     ch = ((plaintext[i] - 'A') + (key[j] - 'A')) % 26;
  else
     ch = plaintext[i] - 'A';  //only encrypt A-Z
  ciphertext.append(string(1, (char)(ch + 'A')));
}

我会改写为：

ciphertext.resize(plaintext.size());
std::transform(plaintext.begin(), plaintext.end(), ciphertext.begin(), MyCrypt(key));

然后把你的地窖功能写成一个很好的函子：

struct MyCrypt
{
            std::string key;
    mutable std::size_t keyIndex;

    MyCrypt(std::string const& key): key(key), keyIndex(0) {}
    char operator()(char const& plain) const
    {
        char keyChar = key[keyIndex] - 'A';
        keyIndex     = (keyIndex + 1) % key.size();

        return (plain >= 'A' && plain <= 'Z')
                   ? (plain - 'A' + keyChar) % 26;
                   : plain - 'A';
    }
};

您的函数

我将更进一步，并使您的函数与任何输入类型一起工作。因此，您基本上是在应用使用迭代器来指定输入/输出的标准约定，并且您的加密也属于这一类。

您的算法不应该关心纯文本来自何处，也不应该关注它的去向。

因此，我也会更改您的函数接口：

template<typename II, typename IO>
void encrypt_vigenere(string const& key, II begin, II end, IO dst)
{
    std::transform(begin, end, dst, MyCrypt(key));
}

现在，当您调用它时，您甚至不需要将内容复制到字符串中，您只需将输入加密到输出中：

encrypt_vigenere("My long key that nobody well guess",
                 std::istreambuf_iterator<char>(std::cin),
                 std::istreambuf_iterator<char>(),
                 std::ostream_iterator<char>(std::cout)
                );

更新:编写函子的正确方法：

// Alternative Version 1:
struct MyCrypt
{
    std::string  key;
    std::size_t& keyIndex;

    MyCrypt(std::string const& key, std::size_t& keyIndex)
        : key(key)
        , keyIndex(keyIndex)
    {
        keyIndex = 0;
    }
    char operator()(char const& plain) const
    {
        char keyChar = key[keyIndex] - 'A';
        keyIndex     = (keyIndex + 1) % key.size();

        return (plain >= 'A' && plain <= 'Z')
                   ? (plain - 'A' + keyChar) % 26;
                   : plain - 'A';
    }
};


// Usage:
std::size_t  keyIndex;
std::transform(begin, end, dst, MyCrypt(key, keyIndex));

或者：

// Alternative Version 2
struct MyCrypt
{
    std::string  key;
    std::size_t  keyIndex;

    MyCrypt(std::string const& key)
        : key(key)
        , keyIndex(0)
    {}
    char operator()(char const& plain)
    {
        char keyChar = key[keyIndex] - 'A';
        keyIndex     = (keyIndex + 1) % key.size();

        return (plain >= 'A' && plain <= 'Z')
                   ? (plain - 'A' + keyChar) % 26;
                   : plain - 'A';
    }
};


// Usage:
MyCrypt  crypt(key);
std::transform(begin, end, dst, crypt);

严格地说，这两种说法都是正确的。

但我发现两者都有缺点。第一个版本要求将索引保持在函子之外(这样函子可以在变异keyIndex时保留其const(ness) )。第二个版本要求您移除operator()的const(ness)，这也意味着您不能动态地创建和传递它作为参数(因为它是一个临时参数，因此我们必须是const)。相反，您需要创建一个对象并传递该对象。

因此，我更喜欢使用我最初展示的方法(它扭曲了规则)，但在我看来，它使代码更易于阅读/维护和理解。

Answer 2

返回值

我将修改加密和解密函数以返回字符串值，而不是接受字符串引用。

从设计的角度来看，这带来了以下几个优点：

1. 使用引用来提供除空字符串之外的任何输入都是最有意义的，因此我将在参数列表中对此进行编码。
2. 任何值得其salt的编译器都将应用返回值优化，因此不应该影响性能。
3. 它消除了呼叫网站的混乱，提高了可读性.

串级联

首选

ciphertext += (char)(ch + 'A');

结束

ciphertext.append(string(1, (char)(ch + 'A')));

对于任何合理的实现，应该没有性能差异。

以下是修改后的代码：

string encrypt_vigenere(const string& plaintext, const string& key){
    string ciphertext;
    int i, j, ch;
    for(i = 0, j = 0; i < plaintext.length(); ++i, j++) {
        if(j >= key.length()) 
            j = 0;
        if(plaintext[i] >= 'A' && plaintext[i] <= 'Z')
            ch = ((plaintext[i] - 'A') + (key[j] - 'A')) % 26;
        else
            ch = plaintext[i] - 'A';  //only encrypt A-Z
        ciphertext += (char)(ch + 'A');
     }
     return ciphertext;
}

string decrypt_vigenere(const string& ciphertext, const string& key){
    string plaintext;
    int i, j, ch;
    for(i = 0, j = 0; i < ciphertext.length(); ++i, j++) {
        if(j >= key.length()) 
            j = 0;

        if(ciphertext[i] >= 'A' && ciphertext[i] <= 'Z')
            ch = ((ciphertext[i] - 'A') + 26 - (key[j] - 'A')) % 26;
        else
            ch = ciphertext[i] - 'A';  //only decrypt A-Z

        plaintext += (char)(ch + 'A');
     }
     return plaintext;
}


int main(int argc, char* argv[])
{
    cout << "Enter plaintext to encrypt:\n";
    char plaintext[256] = {0};
    char key[256] = {0};
    cin.getline (plaintext,256);
    cout << "Text to encrypt: " << plaintext << endl;

    cin.clear();  //clears any cin errors 

    cout << "Enter key (can be any string):";

     cin.getline (key, 256);

     //uppercase KEY
     transform(key, key+strlen(key), key, to_upper());

    cout << "key chosen: " << key << endl;

    transform(plaintext, plaintext+strlen(plaintext), plaintext, to_upper());
    string cipher = encrypt_vigenere(plaintext, key);

    string decrypted = decrypt_vigenere(cipher, key);

    cout << "Vigenere encrypted text: " << cipher << " decrypted: " << decrypted << endl;

    return 0;
}

更次要的一点是，我不太喜欢这样的评论：

//uppercase KEY
transform(key, key+strlen(key), key, to_upper());

这些评论应该解释为什么，而不是什么(这已经不言自明)。我更喜欢评论，解释为什么你总是返回大写的密钥和密码。

Answer 3

除了cmh和Loki说过的话外，这里还有我的一些小毛病：

1. 从你的代码库中进行C风格的转换。他们是危险的(他们是一个无所不包的演员，严重破坏了类型系统)，太无害了。演员们应该脱颖而出。所以，不要使用(char) foo，而是使用static_cast<char>(foo)；是的，它又长又丑。这是有意让演员们脱颖而出的。
2. 将逻辑分解为更小的非重复块。对于Loki的修改，这是过时的，但总的来说，看看您的加密和解密功能是多么相似。您应该努力减少这种冗余。如果我正确地记得我的Vigenère，这些函数是对称的，可以用相同的逻辑实现。通过添加或减去'A'，您还可以在字符和数字之间进行大量转换。将其封装到一对函数中，可以使代码更加简洁。